[Bug]Fix the bug data balance causes tablet loss by qidaye · Pull Request #6063 · apache/doris

qidaye · 2021-06-18T10:36:10Z

Proposed changes

fix #6061

Add version information in BE tablet meta, which is replica_id in FE meta.

Add replica_id information in CloneTask/CreateReplicaTask/DropReplicaTask of FE.

There are 4 main changes in BE:

add replica_id information when creating a tablet
clone tablet with checking replica_id, and add in_clone_mode flag in tablet_meta
check version information(replica_id) and in_clone_mode before drop a tablet
add replica_id information when reporting tablet to FE

Two other issues have also been considered.

Do not affect other tasks.
Compatibility of old and new data, with and without version information.

Types of changes

What types of changes does your code introduce to Doris?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)
Code refactor (Modify the code structure, format the code, etc...)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have created an issue on (Fix [Bug] Tablet is lost due to data balancing #6061) and described the bug/feature there in detail
Compiling and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
If these changes need document changes, I have updated the document
Any dependent changes have been merged

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

morningman · 2021-07-12T05:48:34Z

be/src/agent/task_worker_pool.cpp

            // get path hash of the created tablet
            TabletSharedPtr tablet = StorageEngine::instance()->tablet_manager()->get_tablet(
-                    create_tablet_req.tablet_id, create_tablet_req.tablet_schema.schema_hash);
+                    create_tablet_req.tablet_id, create_tablet_req.replica_id, create_tablet_req.tablet_schema.schema_hash);


Check if create_tablet_req.replica_id is set before using it. We can not rely on the default value of it. It depends on thrift's implementation. Or you can set the default value in thrift file.

morningman · 2021-07-12T05:50:25Z

be/src/agent/task_worker_pool.cpp

+        TabletSharedPtr exist_tablet = StorageEngine::instance()->tablet_manager()->get_tablet(
+                clone_req.tablet_id, 0 /*replica_id*/, clone_req.schema_hash, &err);
+        if (exist_tablet != nullptr) {
+            exist_tablet->set_clone_mode(true);


Why not checking replica id here?

morningman · 2021-07-12T05:50:54Z

be/src/agent/task_worker_pool.cpp

+        // clone done, set clone mode false
+        // Retrieve once again to prevent tablet from being dropped
+        exist_tablet = StorageEngine::instance()->tablet_manager()->get_tablet(
+                clone_req.tablet_id, 0 /*replica_id*/, clone_req.schema_hash, &err);


Why not checking replica id here?

morningman · 2021-07-12T07:01:32Z

be/src/olap/task/engine_clone_task.cpp

-                        StorageEngine::instance()->tablet_manager()->load_tablet_from_dir(
+                OLAPStatus load_header_status;
+                if (old_version_tablet != nullptr) {
+                    // drop old version tablet first, then and new tablet


Suggested change

// drop old version tablet first, then and new tablet

// drop old version tablet first, then add new tablet

morningman · 2021-07-12T07:38:46Z

be/src/olap/task/engine_clone_task.cpp

-                LOG(WARNING) << "errors while set tablet uid: '" << header_path;
-                _error_msgs->push_back("errors while set tablet uid.");
+            // reset_replica_id here. before load tablet to tablet_manager
+            OLAPStatus reset_replica_id_status = TabletMeta::reset_tablet_replica_id(header_path, _clone_req.replica_id);


merge reset_tablet_replica_id() into reset_tablet_uid()? To avoid save meta twice?

morningman · 2021-08-06T03:59:57Z

be/src/agent/task_worker_pool.cpp

        if (dropped_tablet != nullptr) {
+            if (dropped_tablet->clone_mode()) {
+                LOG(WARNING) << "drop table cancelled as tablet is in clone mode! signature: " << agent_task_req.signature;
+                error_msgs.push_back("drop table cancelled!");


add reason to error_msgs too.

Change-Id: Ibfb98ccf52966f6995f55f7ccb0e470abe347e29

morningman · 2021-11-11T14:40:59Z

gensrc/thrift/AgentService.thrift

    13: optional TStorageFormat storage_format
    14: optional TTabletType tablet_type
+    15: optional Types.TReplicaId replica_id
+    16: optional Types.TReplicaId base_replica_id


Add comment to explain replica_id and base_replica_id

morningman · 2021-11-11T14:45:59Z

be/src/agent/task_worker_pool.cpp

-                drop_tablet_req.tablet_id, drop_tablet_req.schema_hash, false, &err);
+                drop_tablet_req.tablet_id, drop_tablet_req.schema_hash, replica_id, false, &err);
        if (dropped_tablet != nullptr) {
+            if (dropped_tablet->clone_mode()) {


Why not just do this check in tablet_manager()->drop_tablet?

morningman · 2021-11-11T14:47:01Z

be/src/agent/task_worker_pool.cpp


+        TReplicaId replica_id = clone_req.__isset.replica_id ? clone_req.replica_id : 0;
+        // check tablet with the same tabletId existance, if exist, set tablet in clone mode
+        TabletSharedPtr exist_tablet = StorageEngine::instance()->tablet_manager()->get_tablet(


I think we can do this in EngineCloneTask.

morningman · 2021-11-11T14:47:29Z

be/src/agent/task_worker_pool.cpp

        task_status.__set_error_msgs(error_msgs);
        finish_task_request.__set_task_status(task_status);

+        // clone done, set clone mode false


Do this in clone task.

1. Provide a FE conf to test the reliability in single replica case when tablet scheduling are frequent. 2. According to #6063, almost apply this fix on current code.

qidaye · 2022-06-15T02:52:33Z

fix at #9971

morningman added area/balance Issues or PRs related to data balance kind/fix Categorizes issue or PR as related to a bug. labels Jun 28, 2021

morningman reviewed Aug 6, 2021

View reviewed changes

qidaye and others added 3 commits October 27, 2021 11:34

[Bug]Fix the bug data balance causes tablet loss

3fef840

set replica_id when restore snapshot

25194fa

Change-Id: Ibfb98ccf52966f6995f55f7ccb0e470abe347e29

fix reviews

3edec7f

qidaye force-pushed the tablet_lost branch from d32c004 to 3edec7f Compare October 27, 2021 12:44

qidaye added 4 commits November 5, 2021 16:45

set replica_id default value to 0; add agentTask req replica_id check

b7915fc

add replica_id status

5787a9d

optimize replica_id check and logs

9b923ff

merge reset tablet_uid and replica_id function to one

92a2606

morningman reviewed Nov 11, 2021

View reviewed changes

morningman self-assigned this Jan 6, 2022

platoneko mentioned this pull request Jun 6, 2022

[Bugfix] Fix the bug data balance causes tablet loss #9971

Merged

qidaye closed this Jun 15, 2022

realize096 mentioned this pull request Dec 27, 2023

[Snyk] Security upgrade axios from 0.19.2 to 1.6.3 realize096/doris#53

Open

chncaption mentioned this pull request Dec 28, 2023

[Snyk] Security upgrade axios from 0.19.2 to 1.6.3 chncaption/doris#61

Open

realize096 mentioned this pull request Jan 5, 2024

[Snyk] Security upgrade axios from 0.19.2 to 1.6.4 realize096/doris#58

Open

chncaption mentioned this pull request Jan 6, 2024

[Snyk] Security upgrade axios from 0.19.2 to 1.6.4 chncaption/doris#67

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]Fix the bug data balance causes tablet loss#6063

[Bug]Fix the bug data balance causes tablet loss#6063
qidaye wants to merge 7 commits intoapache:masterfrom
qidaye:tablet_lost

qidaye commented Jun 18, 2021

Uh oh!

morningman Jul 12, 2021

Uh oh!

morningman Jul 12, 2021

Uh oh!

morningman Jul 12, 2021

Uh oh!

morningman Jul 12, 2021

Uh oh!

morningman Jul 12, 2021

Uh oh!

morningman Aug 6, 2021

Uh oh!

morningman Nov 11, 2021

Uh oh!

morningman Nov 11, 2021

Uh oh!

morningman Nov 11, 2021

Uh oh!

morningman Nov 11, 2021

Uh oh!

qidaye commented Jun 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// drop old version tablet first, then and new tablet
	// drop old version tablet first, then add new tablet

Conversation

qidaye commented Jun 18, 2021

Proposed changes

Types of changes

Checklist

Further comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qidaye commented Jun 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants