Remove TxnMergeTree from the Raft layer by JaySon-Huang · Pull Request #3563 · pingcap/tiflash

JaySon-Huang · 2021-11-29T17:07:01Z

What problem does this PR solve?

Issue Number: a part of #3481

Problem Summary:

What is changed and how it works?

Move BackgroundProcessingPool into Storages level
Do NOT generate TxnMergeTree in the Raft layer
Remove codes for optimizing TxnMergeTree table

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

ti-chi-bot · 2021-11-29T17:07:03Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SchrodingerZhu

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang · 2021-11-29T17:16:51Z

/run-all-tests

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

sre-bot · 2021-11-29T17:40:03Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/212/cobertura/
(Coverage detail url is limited office network access)

lines: 37.5% (44401 out of 118427)
branches: 5.5% (67153 out of 1215349)

JaySon-Huang · 2021-11-29T17:48:23Z

/run-all-tests

sre-bot · 2021-11-29T18:10:34Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/213/cobertura/
(Coverage detail url is limited office network access)

lines: 37.6% (44488 out of 118427)
branches: 5.5% (67238 out of 1215349)

SchrodingerZhu

large change. but LGTM.

JaySon-Huang · 2021-11-30T03:07:31Z

/cc @solotzg @fuzhe1989

JaySon-Huang

The main changes are listed below:

JaySon-Huang · 2021-11-30T03:12:24Z

dbms/src/Storages/Transaction/ApplySnapshot.cpp

        {
            switch (storage->engineType())
            {
-            case TiDB::StorageEngine::TMT:


JaySon-Huang · 2021-11-30T03:12:58Z

dbms/src/Storages/Transaction/RegionTable.cpp

-            if (auto & tmt = context->getTMTContext(); !tmt.isBgFlushDisabled())
-            {
-                /// All regions of this table is removed, the storage maybe drop or pd
-                /// move it to another node, we can optimize outdated data.
-                table_to_optimize.insert(table_id);
-            }


JaySon-Huang · 2021-11-30T03:13:08Z

dbms/src/Storages/Transaction/SchemaBuilder.cpp

-    if (table_info.engine_type == TiDB::StorageEngine::TMT)
-    {
-        writeString(") Engine = TxnMergeTree((", stmt_buf);
-        for (size_t i = 0; i < pks.size(); i++)
-        {
-            if (i > 0)
-                writeString(", ", stmt_buf);
-            writeBackQuotedString(pks[i], stmt_buf);
-        }
-        writeString("), 8192, '", stmt_buf);
-        writeEscapedString(table_info.serialize(), stmt_buf);
-        writeString("')", stmt_buf);
-    }
-    else if (table_info.engine_type == TiDB::StorageEngine::DT)


JaySon-Huang · 2021-11-30T03:13:40Z

dbms/src/Storages/Transaction/BackgroundService.cpp

-            // if all regions of table is removed, try to optimize data.
-            if (auto table_id = region_table.popOneTableToOptimize(); table_id != InvalidTableID)
-            {
-                LOG_INFO(log, "try to final optimize table " << table_id);
-                tryOptimizeStorageFinal(tmt.getContext(), table_id);
-                LOG_INFO(log, "finish final optimize table " << table_id);
-            }


solotzg

lgtm

JaySon-Huang · 2021-11-30T04:17:40Z

/merge

ti-chi-bot · 2021-11-30T04:17:41Z

@JaySon-Huang: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2021-11-30T04:17:46Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 2d12323

sre-bot · 2021-11-30T04:40:06Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/215/cobertura/
(Coverage detail url is limited office network access)

lines: 37.5% (44399 out of 118427)
branches: 5.5% (67155 out of 1215349)

JaySon-Huang added 5 commits November 29, 2021 23:37

Move BackgroundProcessingPool out of MergeTree

0704f1f

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

Cleanup

9574a10

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

Fix GCManager

5e887b4

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

Remove TxnMergeTree from the Raft layer

4c70de9

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

Disable TxnMergeTree

0e19f14

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang added the type/code-quality-improvement PR that can improve the code quality label Nov 29, 2021

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 29, 2021

Format files

89b06f0

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

Fix compile error

2d12323

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang mentioned this pull request Nov 29, 2021

Remove MergeTree family #3564

Merged

12 tasks

SchrodingerZhu approved these changes Nov 30, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 30, 2021

ti-chi-bot requested review from fuzhe1989 and solotzg November 30, 2021 03:07

JaySon-Huang mentioned this pull request Nov 30, 2021

Remove MergeTree family #3481

Closed

6 tasks

JaySon-Huang commented Nov 30, 2021

View reviewed changes

solotzg approved these changes Nov 30, 2021

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 30, 2021

ti-chi-bot merged commit aa23342 into pingcap:master Nov 30, 2021

JaySon-Huang deleted the remove_merge_tree_1 branch November 30, 2021 04:18

Conversation

JaySon-Huang commented Nov 29, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JaySon-Huang commented Nov 29, 2021

Uh oh!

sre-bot commented Nov 29, 2021

Uh oh!

JaySon-Huang commented Nov 29, 2021

Uh oh!

sre-bot commented Nov 29, 2021

Uh oh!

SchrodingerZhu left a comment

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang commented Nov 30, 2021

Uh oh!

JaySon-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

solotzg left a comment

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang commented Nov 30, 2021

Uh oh!

ti-chi-bot commented Nov 30, 2021

Uh oh!

ti-chi-bot commented Nov 30, 2021

Uh oh!

sre-bot commented Nov 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ti-chi-bot commented Nov 29, 2021 •

edited

Loading