fix: executor/scheduler should be latest replica meta but not replica copy #45877
Conversation
… copy Signed-off-by: chyezh <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: chyezh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
mergify
bot
added
dco-passed
|
[ci-v2-notice]
To rerun ci-v2 checks, comment with:
If you have any questions or requests, please contact @zhikunyao. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #45877 +/- ##
==========================================
+ Coverage 76.13% 76.18% +0.05%
==========================================
Files 1869 1881 +12
Lines 292340 294241 +1901
==========================================
+ Hits 222576 224171 +1595
- Misses 62378 62631 +253
- Partials 7386 7439 +53
🚀 New features to boost your workflow:
|
|
/ci-rerun-ut-go |
| log.Warn("node doesn't belong to any replica", zap.Error(err)) | ||
| return | ||
| } | ||
| view := ex.dist.ChannelDistManager.GetShardLeader(task.Shard(), replica) |
There was a problem hiding this comment.
do we need to verify replicaID is same as we execute?
| leader := scheduler.distMgr.ChannelDistManager.GetShardLeader(task.Shard(), task.replica) | ||
| leader := scheduler.getReplicaShardLeader(task.Shard(), task.ReplicaID()) | ||
| if leader == nil { | ||
| return merr.WrapErrServiceInternal("segment's delegator leader not found, stop balancing") |
There was a problem hiding this comment.
do we need to check the target node is still under the replica?
probably it's safe to lock and drain all the balance task before replica change? currently there seems to be many corner cases to handle?
There was a problem hiding this comment.
if we don't want replica mutual exclusive to balance, it might at least to check the replica exist and node belongs to replica at preProcess?
| // wait for new delegator becomes leader, then try to remove old leader | ||
| task := task.(*ChannelTask) | ||
| delegator := scheduler.distMgr.ChannelDistManager.GetShardLeader(task.Shard(), task.replica) | ||
| delegator := scheduler.getReplicaShardLeader(task.Shard(), task.ReplicaID()) |
There was a problem hiding this comment.
is there a possibility that the node does not belong to the replica anymore?
3 participants
issue: #45865