Support ddl job `CreateTables` in TiFlash by CalvinNeo · Pull Request #4201 · pingcap/tiflash

CalvinNeo · 2022-03-08T09:54:46Z

What problem does this PR solve?

Issue Number: close #4194. Related to pingcap/tidb#28763

Problem Summary:

For all ddl jobs need handled before CreateTables, is not addressed in this PR. We only generate an error message.

ref #4215

What is changed and how it works?

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

Support BR restoring tables with ddl job `CreateTables` in TiFlash

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot · 2022-03-08T09:54:47Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JaySon-Huang
windtalker

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.

dbms/src/Storages/Transaction/SchemaGetter.h

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fengou1 · 2022-03-09T09:00:31Z

a manual test show this solution works, steps as follows:

create sql with TiFlash's some table replicas
mysql> alter table t1 set tiflash replica 1;
backup the TiDB cluster
restore to another tidb cluster with TiFlash's
check the TiFlash's table t1, it works

ti-chi-bot · 2022-03-09T09:01:26Z

@fengou1: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

Details

In response to this:

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.

fengou1 · 2022-03-09T09:03:11Z

/run-build

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fengou1 · 2022-03-10T02:51:03Z

/cc @windtalker PTAL

ti-chi-bot · 2022-03-10T02:51:05Z

@fengou1: GitHub didn't allow me to request PR reviews from the following users: PTAL.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @windtalker PTAL

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 kubernetes/test-infra repository.

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

windtalker · 2022-03-10T01:55:50Z

dbms/src/Debug/dbgFuncMockTiDBTable.cpp

+
+    for (ASTs::size_type i = 1; i < args.size(); i++)
+    {
+        String schema_str = "i Int64";


A hard-coded schema for all the tables?

Yes, since newTables is just a delegate of addTable. This ut is to ensure this behavior.
If it can not support other schemas, test for mock_tidb_table shall fail.

dbms/src/Storages/Transaction/SchemaGetter.h

CalvinNeo · 2022-03-10T05:29:24Z

Test passed

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo · 2022-03-10T05:37:07Z

/run-all-tests

CalvinNeo · 2022-03-10T07:38:12Z

/merge

ti-chi-bot · 2022-03-10T07:38:13Z

@CalvinNeo: 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 · 2022-03-10T07:38:19Z

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

Details

Commit hash: 740d6ff

ti-chi-bot · 2022-03-10T07:38:28Z

@CalvinNeo: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sre-bot · 2022-03-10T07:50:51Z

Coverage for changed files

Filename                                   Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Debug/DBGInvoker.cpp                            30                29     3.33%          10                 9    10.00%         138                75    45.65%          14                14     0.00%
Debug/MockTiDB.cpp                             139               135     2.88%          34                32     5.88%         422               404     4.27%          92                91     1.09%
Debug/MockTiDB.h                                 8                 6    25.00%           8                 6    25.00%          15                13    13.33%           0                 0         -
Debug/dbgFuncMockTiDBTable.cpp                 109               109     0.00%          14                14     0.00%         219               219     0.00%          72                72     0.00%
Debug/dbgFuncSchema.cpp                         44                44     0.00%           5                 5     0.00%          62                62     0.00%          28                28     0.00%
Storages/Transaction/SchemaBuilder.cpp         651               616     5.38%          46                42     8.70%        1020               948     7.06%         404               386     4.46%
Storages/Transaction/SchemaBuilder.h             1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
Storages/Transaction/SchemaGetter.h              1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          983               941     4.27%         119               110     7.56%        1878              1723     8.25%         610               591     3.11%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16777      9446             43.70%    188928  95772        49.31%

full coverage report (for internal network access only)

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot · 2022-03-10T08:26:29Z

@CalvinNeo: 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 · 2022-03-10T08:26:35Z

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

Details

Commit hash: 9f4c4e0

sre-bot · 2022-03-10T08:36:57Z

Coverage for changed files

Filename                                   Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Debug/DBGInvoker.cpp                            30                29     3.33%          10                 9    10.00%         138                75    45.65%          14                14     0.00%
Debug/MockTiDB.cpp                             139               135     2.88%          34                32     5.88%         421               403     4.28%          92                91     1.09%
Debug/MockTiDB.h                                 8                 6    25.00%           8                 6    25.00%          15                13    13.33%           0                 0         -
Debug/dbgFuncMockTiDBTable.cpp                 109               109     0.00%          14                14     0.00%         219               219     0.00%          72                72     0.00%
Debug/dbgFuncSchema.cpp                         44                44     0.00%           5                 5     0.00%          62                62     0.00%          28                28     0.00%
Storages/Transaction/SchemaBuilder.cpp         651               616     5.38%          46                42     8.70%        1020               948     7.06%         404               386     4.46%
Storages/Transaction/SchemaBuilder.h             1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
Storages/Transaction/SchemaGetter.h              1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          983               941     4.27%         119               110     7.56%        1877              1722     8.26%         610               591     3.11%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16777      9447             43.69%    188928  95758        49.32%

full coverage report (for internal network access only)

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo · 2022-03-10T08:45:58Z

/merge

ti-chi-bot · 2022-03-10T08:45:58Z

@CalvinNeo: 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 · 2022-03-10T08:46:05Z

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

Details

Commit hash: abdae32

sre-bot · 2022-03-10T08:56:25Z

Coverage for changed files

Filename                                   Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Debug/DBGInvoker.cpp                            30                29     3.33%          10                 9    10.00%         138                75    45.65%          14                14     0.00%
Debug/MockTiDB.cpp                             139               135     2.88%          34                32     5.88%         421               403     4.28%          92                91     1.09%
Debug/MockTiDB.h                                 8                 6    25.00%           8                 6    25.00%          15                13    13.33%           0                 0         -
Debug/dbgFuncMockTiDBTable.cpp                 109               109     0.00%          14                14     0.00%         219               219     0.00%          72                72     0.00%
Debug/dbgFuncSchema.cpp                         44                44     0.00%           5                 5     0.00%          62                62     0.00%          28                28     0.00%
Storages/Transaction/SchemaBuilder.cpp         651               616     5.38%          46                42     8.70%        1020               948     7.06%         404               386     4.46%
Storages/Transaction/SchemaBuilder.h             1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
Storages/Transaction/SchemaGetter.h              1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          983               941     4.27%         119               110     7.56%        1877              1722     8.26%         610               591     3.11%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16777      9447             43.69%    188928  95743        49.32%

full coverage report (for internal network access only)

CalvinNeo · 2022-03-10T09:17:53Z

/run-integration-tests

CalvinNeo · 2022-03-10T11:35:10Z

/run-integration-tests

CalvinNeo · 2022-03-10T14:49:31Z

/run-integration-tests

fengou1 · 2022-03-11T02:27:54Z

/run-integration-test

JaySon-Huang · 2022-03-11T06:11:05Z

/run-integration-test

JaySon-Huang · 2022-03-11T07:19:27Z

/run-integration-test

CalvinNeo added 2 commits March 8, 2022 16:28

fix

a3fd226

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fix

7655552

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Mar 8, 2022

CalvinNeo changed the title ~~Fix4194~~ Handle CreateTables ddl job Mar 8, 2022

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 8, 2022

windtalker reviewed Mar 9, 2022

View reviewed changes

dbms/src/Storages/Transaction/SchemaGetter.h Show resolved Hide resolved

try add test

bd906c7

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 9, 2022

add test

c83d9f2

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

fengou1 approved these changes Mar 9, 2022

View reviewed changes

CalvinNeo added 2 commits March 9, 2022 18:51

f

484d606

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

addr clang tidy

58dabde

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot requested a review from windtalker March 10, 2022 02:51

add

05749be

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

windtalker reviewed Mar 10, 2022

View reviewed changes

add warning

3bffbdd

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

CalvinNeo changed the title ~~Handle CreateTables ddl job~~ Support ddl job CreateTables in TiFlash Mar 10, 2022

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 10, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 10, 2022

Merge branch 'master' into fix4194

d1f856f

CalvinNeo added 2 commits March 10, 2022 16:25

remove lock

99a0bc8

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

Merge branch 'fix4194' of github.com:CalvinNeo/tics into fix4194

9f4c4e0

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 10, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 10, 2022

f

abdae32

Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 10, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 10, 2022

ti-chi-bot merged commit b887653 into pingcap:master Mar 11, 2022

Conversation

CalvinNeo commented Mar 8, 2022 • edited by JaySon-Huang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fengou1 commented Mar 9, 2022

Uh oh!

ti-chi-bot commented Mar 9, 2022

Uh oh!

fengou1 commented Mar 9, 2022

Uh oh!

fengou1 commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

windtalker Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

CalvinNeo Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

sre-bot commented Mar 10, 2022

Coverage for changed files

Coverage summary

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

sre-bot commented Mar 10, 2022

Coverage for changed files

Coverage summary

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

ti-chi-bot commented Mar 10, 2022

Uh oh!

sre-bot commented Mar 10, 2022

Coverage for changed files

Coverage summary

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

CalvinNeo commented Mar 10, 2022

Uh oh!

fengou1 commented Mar 11, 2022

Uh oh!

JaySon-Huang commented Mar 11, 2022

Uh oh!

JaySon-Huang commented Mar 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

CalvinNeo commented Mar 8, 2022 •

edited by JaySon-Huang

Loading

ti-chi-bot commented Mar 8, 2022 •

edited

Loading