cluster: reset stores cache before loading cluster info #4942

rleungx · 2022-05-13T03:01:33Z

Signed-off-by: Ryan Leung [email protected]

What problem does this PR solve?

Issue Number: Close #4941.

What is changed and how does it work?

This PR resets the new leader store cache after transferring the PD leader.

Check List

Tests

Unit test

Release note

Fix the issue that the removed tombstone store shows again after transferring the PD leader

Signed-off-by: Ryan Leung <[email protected]>

ti-chi-bot · 2022-05-13T03:01:35Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

AndreMouche
CabinfeverB

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.

codecov · 2022-05-13T03:07:34Z

Codecov Report

Merging #4942 (e419e6d) into master (d6557df) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4942      +/-   ##
==========================================
+ Coverage   75.18%   75.24%   +0.05%     
==========================================
  Files         297      297              
  Lines       29492    29496       +4     
==========================================
+ Hits        22173    22193      +20     
+ Misses       5369     5356      -13     
+ Partials     1950     1947       -3

Flag	Coverage Δ
unittests	`75.24% <100.00%> (+0.05%)`	⬆️

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

Impacted Files	Coverage Δ
server/cluster/cluster.go	`83.79% <100.00%> (+1.05%)`	⬆️
server/core/basic_cluster.go	`91.14% <100.00%> (+0.14%)`	⬆️
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
server/storage/leveldb_backend.go	`76.00% <0.00%> (-8.00%)`	⬇️
server/id/id.go	`76.19% <0.00%> (-4.77%)`	⬇️
server/member/member.go	`62.10% <0.00%> (-3.69%)`	⬇️
pkg/etcdutil/etcdutil.go	`82.75% <0.00%> (-2.30%)`	⬇️
client/base_client.go	`79.89% <0.00%> (-2.07%)`	⬇️
server/schedulers/utils.go	`92.53% <0.00%> (-1.50%)`	⬇️
server/region_syncer/server.go	`79.89% <0.00%> (-1.09%)`	⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6557df...e419e6d. Read the comment docs.

AndreMouche

LGTM

CabinfeverB · 2022-05-13T03:40:48Z

tests/server/cluster/cluster_test.go

+	svr = leaderServer.GetServer()
+	rc = cluster.NewRaftCluster(s.ctx, svr.ClusterID(), syncer.NewRegionSyncer(svr), svr.GetClient(), svr.GetHTTPClient())
+	rc.InitCluster(svr.GetAllocator(), svr.GetPersistOptions(), svr.GetStorage(), svr.GetBasicCluster())
+	raftCluster, err = rc.LoadClusterInfo()


IMO, it is best not to call LoadClusterInfo explicitly

Suggested change

raftCluster, err = rc.LoadClusterInfo()

newStores := svr.GetBasicCluster().GetStores()

// check store count

c.Assert(newStores, HasLen, 3)

Signed-off-by: Ryan Leung <[email protected]>

nolouch · 2022-05-13T09:04:59Z

/merge

ti-chi-bot · 2022-05-13T09:05:00Z

@nolouch: 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 · 2022-05-13T09:05:01Z

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

Commit hash: c9275ba

ti-chi-bot · 2022-05-16T04:10:46Z

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

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-05-16T04:21:13Z

In response to a cherrypick label: new pull request created: #4955.

ti-chi-bot · 2022-05-16T04:21:48Z

In response to a cherrypick label: new pull request created: #4956.

ti-chi-bot · 2022-05-16T04:22:23Z

In response to a cherrypick label: new pull request created: #4957.

close tikv#4941 Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2022-05-16T04:22:59Z

In response to a cherrypick label: new pull request created: #4958.

close tikv#4941 Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2022-05-16T04:23:35Z

In response to a cherrypick label: new pull request created: #4959.

close #4941, ref #4942 Signed-off-by: ti-chi-bot <[email protected]> Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

close #4941, ref #4942 Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

close #4941, ref #4942 Signed-off-by: ti-chi-bot <[email protected]> Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

close #4941, ref #4942 Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

reset stores cache before loading cluster info

08d397c

Signed-off-by: Ryan Leung <[email protected]>

rleungx added the component/cluster Cluster logic. label May 13, 2022

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/needs-triage-completed labels May 13, 2022

ti-chi-bot requested review from disksing and Yisaer May 13, 2022 03:01

AndreMouche approved these changes May 13, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 13, 2022

CabinfeverB reviewed May 13, 2022

View reviewed changes

address the comment

e03fa96

Signed-off-by: Ryan Leung <[email protected]>

CabinfeverB approved these changes May 13, 2022

View reviewed changes

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 May 13, 2022

Merge branch 'master' into fix-issue-4941

c9275ba

nolouch approved these changes May 13, 2022

View reviewed changes

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

nolouch removed the do-not-merge/needs-triage-completed label May 13, 2022

Merge branch 'master' into fix-issue-4941

69ea3e8

ti-chi-bot added the needs-cherry-pick-release-6.0 The PR needs to cherry pick to release-6.0 branch. label May 16, 2022

ti-chi-bot added 2 commits May 16, 2022 10:16

Merge branch 'master' into fix-issue-4941

663667e

Merge branch 'master' into fix-issue-4941

e419e6d

rleungx removed the needs-cherry-pick-release-6.0 The PR needs to cherry pick to release-6.0 branch. label May 16, 2022

ti-chi-bot merged commit d127054 into tikv:master May 16, 2022

ti-chi-bot mentioned this pull request May 16, 2022

cluster: reset stores cache before loading cluster info (#4942) #4955

Merged

ti-chi-bot mentioned this pull request May 16, 2022

cluster: reset stores cache before loading cluster info (#4942) #4956

Merged

ti-chi-bot mentioned this pull request May 16, 2022

cluster: reset stores cache before loading cluster info (#4942) #4957

Merged

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 16, 2022

This is an automated cherry-pick of tikv#4942

e063d36

close tikv#4941 Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request May 16, 2022

cluster: reset stores cache before loading cluster info (#4942) #4958

Merged

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 16, 2022

This is an automated cherry-pick of tikv#4942

cb1bb29

close tikv#4941 Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request May 16, 2022

cluster: reset stores cache before loading cluster info (#4942) #4959

Merged

rleungx deleted the fix-issue-4941 branch May 17, 2022 10:14

ti-chi-bot added a commit that referenced this pull request Jun 14, 2022

cluster: reset stores cache before loading cluster info (#4942) (#4956)

0b84f98

close #4941, ref #4942 Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

ti-chi-bot added a commit that referenced this pull request Jul 18, 2022

cluster: reset stores cache before loading cluster info (#4942) (#4955)

92c41c0

close #4941, ref #4942 Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

ti-chi-bot added a commit that referenced this pull request Sep 21, 2022

cluster: reset stores cache before loading cluster info (#4942) (#4957)

c7dcc3c

close #4941, ref #4942 Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cluster: reset stores cache before loading cluster info #4942

cluster: reset stores cache before loading cluster info #4942

rleungx commented May 13, 2022

ti-chi-bot commented May 13, 2022 •

edited

Loading

codecov bot commented May 13, 2022 •

edited

Loading

AndreMouche left a comment

CabinfeverB May 13, 2022

nolouch commented May 13, 2022

ti-chi-bot commented May 13, 2022

ti-chi-bot commented May 13, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

-	raftCluster, err = rc.LoadClusterInfo()
+	newStores := svr.GetBasicCluster().GetStores()
+	// check store count
+	c.Assert(newStores, HasLen, 3)

cluster: reset stores cache before loading cluster info #4942

cluster: reset stores cache before loading cluster info #4942

Conversation

rleungx commented May 13, 2022

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented May 13, 2022 • edited Loading

codecov bot commented May 13, 2022 • edited Loading

Codecov Report

AndreMouche left a comment

Choose a reason for hiding this comment

CabinfeverB May 13, 2022

Choose a reason for hiding this comment

nolouch commented May 13, 2022

ti-chi-bot commented May 13, 2022

ti-chi-bot commented May 13, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 16, 2022

ti-chi-bot commented May 13, 2022 •

edited

Loading

codecov bot commented May 13, 2022 •

edited

Loading