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: support creating tiflash replica in API V2 #41415

Merged
merged 29 commits into from
Mar 14, 2023
Merged

ddl: support creating tiflash replica in API V2 #41415

merged 29 commits into from
Mar 14, 2023

Conversation

iosmanthus
Copy link
Contributor

@iosmanthus iosmanthus commented Feb 14, 2023 •
edited
Loading

Signed-off-by: iosmanthus myosmanthustree@gmail.com

What problem does this PR solve?

Issue Number: close #41414

Deps:

Problem Summary:

What is changed and how it works?

This pull request encodes the placement rules for API v2 user and uses a new HTTP API of TiFlash to query the availability of replica: /sync-status/keyspace/xxx/table/xxx.

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

  • Affects user behaviours
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

ddl: support creating tiflash replica in API V2

@iosmanthus
support creating tiflash replica
a13553c 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
Merge branch 'master' of github.com:pingcap/tidb into keyspace-tiflash
571ecf1 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 14, 2023 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • tangenta
  • wjhuang2016

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 release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 14, 2023
@iosmanthus
make linter happy
1406f13 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
fix test compile error
af6fe72 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
fix race of attachAPICtx
5a73b10 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
make bazel_prepare
7c18d8d 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
Copy link
Contributor Author

/test unit-test

@iosmanthus
update client-go
7273953 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
bump kvproto to master
73146de 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 16, 2023
@iosmanthus
bump client-go to master
42fc2a8 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
make bazel_prepare
5026582 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
fix keyspace tiflash rule is not GCed
9d82a4b 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2023
@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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 7, 2023
@purelind
Copy link
Contributor

purelind commented Mar 7, 2023

/retest

@iosmanthus
bump kvproto to master
47c9020 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@iosmanthus
Merge branch 'master' of github.com:pingcap/tidb into keyspace-tiflash
0477ed9 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 10, 2023
@iosmanthus
update client-go
76b53c0 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 13, 2023
@iosmanthus
Copy link
Contributor Author

/test unit-test

CalvinNeo
CalvinNeo reviewed Mar 13, 2023
View reviewed changes
domain/infosync/tiflash_manager.go

return &ruleNew
// MakeRuleID creates a rule ID for TiFlash with given TableID.
Copy link
Member

Choose a reason for hiding this comment

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

So this function is used for both API v1 and v2, and in v2 it will be decorated by encodeRuleID? If so, maybe we can elaborate comments here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@iosmanthus
add some comments for new tiflash api
84c7c16 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
CalvinNeo
CalvinNeo approved these changes Mar 13, 2023
View reviewed changes
Copy link
Member

@CalvinNeo CalvinNeo 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

@CalvinNeo: 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.

In response to this:

lgtm

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.

@iosmanthus
add comments for MakeRuleID and unify MakeRuleID usage
db349f5 
Signed-off-by: iosmanthus <myosmanthustree@gmail.com>
@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 Mar 13, 2023
CalvinNeo
CalvinNeo approved these changes Mar 13, 2023
View reviewed changes
Copy link
Member

@CalvinNeo CalvinNeo 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

@CalvinNeo: 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.

In response to this:

lgtm

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.

@tangenta
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: db349f5

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 13, 2023
@iosmanthus
Copy link
Contributor Author

/retest

2 similar comments
@iosmanthus
Copy link
Contributor Author

/retest

@iosmanthus
Copy link
Contributor Author

/retest

@iosmanthus
Copy link
Contributor Author

/test unit-test

@iosmanthus
Copy link
Contributor Author

/retest

@iosmanthus
Copy link
Contributor Author

/rebuild

@hawkingrei
Copy link
Member

/retest

@wuhuizuo
Copy link
Contributor

/retest all

@ti-chi-bot
Copy link
Member

@wuhuizuo: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test build
  • /test check-dev
  • /test check-dev2
  • /test mysql-test
  • /test unit-test

Use /test all to run all jobs.

In response to this:

/retest all

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.

@wuhuizuo
Copy link
Contributor

/test all

@iosmanthus
Copy link
Contributor Author

/retest

@iosmanthus
Copy link
Contributor Author

/rebuild

@wuhuizuo
Copy link
Contributor

/retest

@ti-chi-bot ti-chi-bot merged commit d10b1c4 into pingcap:master Mar 14, 2023
@iosmanthus iosmanthus deleted the keyspace-tiflash branch May 26, 2023 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CalvinNeo CalvinNeo CalvinNeo approved these changes

@wjhuang2016 wjhuang2016 wjhuang2016 approved these changes

@tangenta tangenta tangenta approved these changes

Assignees
No one assigned
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 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.

Support TiFlash replica in keyspace mode
9 participants
@iosmanthus @ti-chi-bot @purelind @ystaticy @tangenta @hawkingrei @wuhuizuo @CalvinNeo @wjhuang2016