mcs: fix sync store label #7396
Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
bot
added
the
release-note-none
ti-chi-bot
bot
added
the
size/XS
rleungx
force-pushed
the
fix-sync-label
branch
from
f4c0876 to
eee09c4
Compare
ti-chi-bot
bot
added
size/S
rleungx
requested review from
lhy1024 and
CabinfeverB
and removed request for
disksing and
Yisaer
| @@ -81,7 +81,10 @@ func (w *Watcher) initializeStoreWatcher() error { | |||
| w.basicCluster.PutStore(core.NewStoreInfo(store)) | |||
| return nil | |||
| } | |||
| w.basicCluster.PutStore(origin.Clone(core.SetStoreState(store.GetState(), store.GetPhysicallyDestroyed()))) | |||
| w.basicCluster.PutStore(origin.Clone( | |||
There was a problem hiding this comment.
May server grpc_server's PutStore has the same problem? miss store.GetPhysicallyDestroyed() and others.
can we copy the new store's meta to the replace origin?
There was a problem hiding this comment.
For scheduling server, I think we don't need to because the other fields is not related to scheduling.
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
rleungx
force-pushed
the
fix-sync-label
branch
from
bf6fd29 to
b95e3d6
Compare
Signed-off-by: Ryan Leung <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #7396 +/- ##
==========================================
- Coverage 74.27% 74.26% -0.02%
==========================================
Files 451 451
Lines 48967 49105 +138
==========================================
+ Hits 36372 36466 +94
- Misses 9375 9416 +41
- Partials 3220 3223 +3
Flags with carried forward coverage won't be shown. Click here to find out more. |
ti-chi-bot
bot
added
status/LGT2
Signed-off-by: Ryan Leung <[email protected]>
ti-chi-bot
bot
added
size/M
|
/merge |
|
@rleungx: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests You only need to trigger
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. |
|
This pull request has been accepted and is ready to merge. Commit hash: a24407d
|
ti-chi-bot
bot
added
the
status/can-merge
| @@ -381,8 +381,7 @@ func (c *Cluster) HandleStoreHeartbeat(heartbeat *schedulingpb.StoreHeartbeatReq | |||
| return errors.Errorf("store %v not found", storeID) | |||
| } | |||
|
|
|||
| nowTime := time.Now() | |||
| newStore := store.Clone(core.SetStoreStats(stats), core.SetLastHeartbeatTS(nowTime)) | |||
| newStore := store.Clone(core.SetStoreStats(stats)) | |||
There was a problem hiding this comment.
only remove SetLastHeartbeatTS?
There was a problem hiding this comment.
Yes, StoreStats doesn't belong to the meta.
close tikv#7391, close tikv#7393, close tikv#7394 Signed-off-by: Ryan Leung <[email protected]>
What problem does this PR solve?
Issue Number: Close #7394, close #7393, close #7391
What is changed and how does it work?
This PR fixes the issue that the store label is not correctly synchronized to scheduling server.
Check List
Tests
Release note