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

ddl: refactor bundle[2/2] [6/6] #22415

Merged
merged 3 commits into from
Jul 8, 2021
Merged

ddl: refactor bundle[2/2] [6/6] #22415

merged 3 commits into from
Jul 8, 2021

Conversation

xhebox
Copy link
Contributor

@xhebox xhebox commented Jan 16, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: close #22414

Problem Summary: This PR refactor the placement packages in a way that is describe in Solutions section of the issue.

After separated, the modification is:

  1. GetLeaderDCByBundle is moved as a method of Bundle.
  2. ObjectIDFromGroupID is moved as a method of Bundle.

Check List

Tests

  • Unit test
  • Integration test

Release note

  • No release note

@xhebox xhebox requested a review from a team as a code owner January 16, 2021 08:49
@xhebox xhebox requested review from qw4990 and removed request for a team January 16, 2021 08:49
@xhebox
Copy link
Contributor Author

xhebox commented Jan 16, 2021

/label sig/infra

@ti-srebot ti-srebot added the sig/sql-infra SIG: SQL Infra label Jan 16, 2021
@xhebox xhebox force-pushed the master branch 2 times, most recently from 4b9ef1c to e7de617 Compare January 26, 2021 06:01
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed sig/infra labels Feb 22, 2021
@ti-chi-bot ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 4, 2021
@xhebox xhebox mentioned this pull request Mar 4, 2021
Closed
@zimulala zimulala added the require-LGT3 Indicates that the PR requires three LGTM. label Mar 30, 2021
@xhebox xhebox force-pushed the master branch 3 times, most recently from 7b7bd50 to 5df111d Compare April 14, 2021 06:11
This was referenced Apr 14, 2021
Merged
Merged
Merged
Merged
Merged
@xhebox xhebox changed the title ddl: refactor placement package ddl: refactor bundle[2/2] [6/6] Apr 14, 2021
@xhebox xhebox requested a review from a team April 14, 2021 07:27
@xhebox xhebox force-pushed the master branch 2 times, most recently from 18a7bec to c1942ef Compare April 17, 2021 04:19
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2021
@xhebox
ddl: refactor bundle[2/2]
f145044 
Signed-off-by: xhe <xw897002528@gmail.com>
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 11, 2021
@xhebox xhebox removed the require-LGT3 Indicates that the PR requires three LGTM. label Jun 11, 2021
@xhebox
Copy link
Contributor Author

xhebox commented Jun 11, 2021

LGT3 is removed, due to this PR is splitted into 6 parts. And the other 5 parts has been merged. Now there are only 300 lines left.

@xhebox
ddl: remove multierror
05d7d08 
Signed-off-by: xhe <xw897002528@gmail.com>
@qw4990 qw4990 removed their request for review June 21, 2021 03:27
@xhebox xhebox mentioned this pull request Jul 7, 2021
Closed
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 8, 2021
AilinKid
AilinKid approved these changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid 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-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AilinKid
  • djshow832

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.

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

@ti-chi-bot 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 Jul 8, 2021
@AilinKid
Copy link
Contributor

AilinKid commented Jul 8, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: e4ab0f3

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 8, 2021
@ti-chi-bot ti-chi-bot merged commit 3bbb2c0 into pingcap:master Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AilinKid AilinKid AilinKid approved these changes

@djshow832 djshow832 djshow832 approved these changes

Assignees
No one assigned
Labels
component/expression sig/execution SIG execution sig/sql-infra SIG: SQL Infra size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ddl: refactor placement package
6 participants
@xhebox @ti-chi-bot @AilinKid @djshow832 @zimulala @ti-srebot