Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

planner: fix bug of plan digest is same when cop task store is different (#20054) #20076

Merged
merged 12 commits into from
Sep 21, 2020
Merged

planner: fix bug of plan digest is same when cop task store is different (#20054) #20076

merged 12 commits into from
Sep 21, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #20054 to release-4.0

What problem does this PR solve?

If 2 plans is the same, but the cop tasks were send to different tikv or tiflash store, the plan digest should be different.

Here is an example:

Plan1

+-------------------+----------+--------------+---------------+--------------------------------+                                                                                                                │ "auto_rand_id": 0,
| id                | estRows  | task         | access object | operator info                  |                                                                                                                │ "max_col_id": 2,
+-------------------+----------+--------------+---------------+--------------------------------+                                                                                                                │ "max_idx_id": 1,
| TableReader_5     | 10000.00 | root         |               | data:TableFullScan_4           |                                                                                                                │ "max_cst_id": 0,
| └─TableFullScan_4 | 10000.00 | cop[tiflash] | table:t1      | keep order:false, stats:pseudo |                                                                                                                │ "update_timestamp": 419514096967745540,
+-------------------+----------+--------------+---------------+--------------------------------+

Plan 2

+-------------------+----------+-----------+---------------+--------------------------------+
| id                | estRows  | task      | access object | operator info                  |
+-------------------+----------+-----------+---------------+--------------------------------+
| TableReader_5     | 10000.00 | root      |               | data:TableFullScan_4           |
| └─TableFullScan_4 | 10000.00 | cop[tikv] | table:t1      | keep order:false, stats:pseudo |
+-------------------+----------+-----------+---------------+--------------------------------+

Plan 1 is same as Plan 2 except for the cop task type. So the digest should be different between Plan 1 and Plan 2.

What is changed and how it works?

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • No

Release note

  • Fix issue of plan digest is same when cop task store is different

@crazycs520 @ti-srebot
cherry pick pingcap#20054 to release-4.0
6ec1b8b 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot mentioned this pull request Sep 17, 2020
Merged
@ti-srebot ti-srebot added this to the 4.0.7 milestone Sep 17, 2020
@crazycs520
Copy link
Contributor

/run-all-tests

tangenta
tangenta reviewed Sep 18, 2020
View reviewed changes
Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot
Copy link
Contributor Author

@tangenta,Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments.See the corresponding SIG page for more information. Related SIG: planner(slack).

@crazycs520
Copy link
Contributor

/run-all-tests

winoros
winoros reviewed Sep 21, 2020
View reviewed changes
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 21, 2020
qw4990
qw4990 approved these changes Sep 21, 2020
View reviewed changes
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 21, 2020
@crazycs520
Copy link
Contributor

/run-all-tests

@SunRunAway
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 21, 2020
@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 20092
  • 20094

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@bb7133
Copy link
Member

bb7133 commented Sep 21, 2020

/merge

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@crazycs520
Copy link
Contributor

/run-all-tests

@lzmhhh123 lzmhhh123 modified the milestones: v4.0.7, v4.0.8 Sep 21, 2020
@crazycs520
Copy link
Contributor

/run-all-tests

@crazycs520
Copy link
Contributor

/run-integration-common-test

@crazycs520
Copy link
Contributor

/run-unit-test

@crazycs520
Copy link
Contributor

/run-all-tests

@crazycs520
Copy link
Contributor

/run-integration-common-test
/run-unit-test
/run-sqllogic-test-1

@crazycs520
Copy link
Contributor

/run-integration-compatibility-test

@crazycs520
Copy link
Contributor

/run-unit-test

@crazycs520 crazycs520 added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Sep 21, 2020
@bb7133 bb7133 merged commit 554c94f into pingcap:release-4.0 Sep 21, 2020
@lzmhhh123 lzmhhh123 modified the milestones: v4.0.8, v4.0.7 Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tangenta tangenta tangenta left review comments

@winoros winoros winoros left review comments

@qw4990 qw4990 qw4990 approved these changes

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

Assignees

@crazycs520 crazycs520

Labels
priority/release-blocker This issue blocks a release. Please solve it ASAP. sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/usability type/4.0-cherry-pick
Projects
None yet
Milestone
v4.0.7
Development

Successfully merging this pull request may close these issues.
8 participants
@ti-srebot @crazycs520 @SunRunAway @bb7133 @qw4990 @winoros @tangenta @lzmhhh123