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.

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

*: move out replication and syncer #6418

Merged
merged 2 commits into from
May 29, 2023

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented May 8, 2023

What problem does this PR solve?

Issue Number: Ref #5839.

What is changed and how does it work?

This PR mainly did the following changes:

  • move replication mode to schedule
  • move unsafe recovery to pkg
  • move coordinator and prepare checker to schedule
  • put operator related logic together
  • put scheduler related logic together
  • use opeartor.ClusterInfo to replace schedule.Cluster in most of the code
  • move the corresponding metrics to the right place
  • rename region_syncer to regionsyncer
  • make all existing tests pass by using mockcluster in some places

Once we reach an agreement, I will try to split this PR into pieces.

Most of the jobs are done in the previous PRs. This PR will only move some packages out of server.

Check List

Tests

  • Unit test

Release note

None.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 8, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • CabinfeverB
  • lhy1024

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
Copy link
Contributor

ti-chi-bot bot commented May 8, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels May 8, 2023
@ti-chi-bot ti-chi-bot bot requested review from disksing and Yisaer May 8, 2023 10:39
@rleungx rleungx marked this pull request as ready for review May 9, 2023 03:52
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 9, 2023
@rleungx rleungx requested review from nolouch, JmPotato, lhy1024, bufferflies, CabinfeverB and binshi-bing and removed request for disksing and Yisaer May 9, 2023 03:52
@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.14 ⚠️

Comparison is base (e28ac74) 74.74% compared to head (b46e00b) 74.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6418      +/-   ##
==========================================
- Coverage   74.74%   74.60%   -0.14%     
==========================================
  Files         414      414              
  Lines       42352    42352              
==========================================
- Hits        31655    31597      -58     
- Misses       7915     7964      +49     
- Partials     2782     2791       +9     
Flag Coverage Δ
unittests 74.60% <ø> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/replication/replication_mode.go 81.57% <ø> (ø)
pkg/syncer/client.go 88.14% <ø> (ø)
pkg/syncer/history_buffer.go 78.46% <ø> (ø)
pkg/syncer/metrics.go 100.00% <ø> (ø)
pkg/syncer/server.go 87.36% <ø> (ø)
server/cluster/cluster.go 80.91% <ø> (-0.48%) ⬇️
server/server.go 75.42% <ø> (+0.11%) ⬆️

... and 32 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2023
ti-chi-bot bot pushed a commit that referenced this pull request May 18, 2023
ref #5839, ref #6418

Signed-off-by: Ryan Leung <rleungx@gmail.com>
ti-chi-bot bot added a commit that referenced this pull request May 22, 2023
)

ref #5839, ref #6418

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this pull request May 23, 2023
ref #5839, ref #6418, ref #6489

Signed-off-by: Ryan Leung <rleungx@gmail.com>
ti-chi-bot bot added a commit that referenced this pull request May 23, 2023
ref #5839, ref #6418, ref #6490

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot added a commit that referenced this pull request May 25, 2023
ref #5839, ref #6418

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Signed-off-by: Ryan Leung <rleungx@gmail.com>
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2023
@rleungx rleungx changed the title *: refactor coordinator *: move out replication and syncer May 26, 2023
@@ -151,7 +151,7 @@ func TestRegionSyncer(t *testing.T) {
re.Equal(region.GetLeader(), r.GetLeader())
re.Equal(region.GetBuckets(), r.GetBuckets())
}
re.NoError(failpoint.Disable("github.com/tikv/pd/server/syncer/noFastExitSync"))
Copy link
Member

Choose a reason for hiding this comment

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

This issue may not be related to this PR change. The old path github.com/tikv/pd/server/syncer/noFastExitSync seems to be wrong? Should be regionsyncer other than syncer. So does this failpoint work?

Copy link
Member Author

Choose a reason for hiding this comment

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

The package name is syncer I think.

Copy link
Member

Choose a reason for hiding this comment

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

Get

@ti-chi-bot ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 26, 2023
@ti-chi-bot ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 29, 2023
@rleungx
Copy link
Member Author

rleungx commented May 29, 2023

/merge

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 29, 2023

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

/run-all-tests

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
Copy link
Contributor

ti-chi-bot bot commented May 29, 2023

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

Commit hash: 78f8c95

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

ti-chi-bot bot commented May 29, 2023

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

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.

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 ti-chi-bot bot merged commit 778d6aa into tikv:master May 29, 2023
@rleungx rleungx deleted the refactor-coordinator branch May 29, 2023 04:43
rleungx added a commit to rleungx/pd that referenced this pull request Dec 1, 2023
rleungx added a commit to rleungx/pd that referenced this pull request Dec 1, 2023
…kv#6489)

ref tikv#5839, ref tikv#6418

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
rleungx added a commit to rleungx/pd that referenced this pull request Dec 1, 2023
ref tikv#5839, ref tikv#6418, ref tikv#6489

Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to rleungx/pd that referenced this pull request Dec 1, 2023
ref tikv#5839, ref tikv#6418, ref tikv#6490

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
rleungx added a commit to rleungx/pd that referenced this pull request Dec 1, 2023
ref tikv#5839, ref tikv#6418

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
rleungx added a commit to rleungx/pd that referenced this pull request Dec 1, 2023
ref tikv#5839

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none Denotes a PR that doesn't merit a release note. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants