hot_region: check leader during handle expired item #9372
Conversation
Signed-off-by: okJiang <[email protected]>
|
Skipping CI for Draft Pull Request. |
ti-chi-bot
bot
added
do-not-merge/needs-triage-completed
do-not-merge/work-in-progress
|
/retest |
|
/cc @lhy1024 |
|
/retest |
ti-chi-bot
bot
removed
the
do-not-merge/work-in-progress
Signed-off-by: okJiang <[email protected]>
ti-chi-bot
bot
added
size/M
| c.GetHotStat().CheckWriteAsync(checkExpiredTask) | ||
| c.GetHotStat().CheckReadAsync(checkExpiredTask) | ||
| c.GetHotStat().CheckReadAsync(checkLeaderChangeTask) |
There was a problem hiding this comment.
Should we check it for Write?
There was a problem hiding this comment.
Is it possible to reuse checkWritePeerTask?
There was a problem hiding this comment.
I'm not sure if need to do this check with write.
There was a problem hiding this comment.
I think justTransferLeader handles the leader change case in CheckPeerFlow, so it shouldn't run checkLeaderChangeTask on Write anymore.
Maybe we should strip out the relevant logic from CheckPeerFlow. But the code(CheckPeerFlow) is kind of crappy right now, and I don't really think of a better way to abstract it away.
There was a problem hiding this comment.
It may need to be checked with write.
There was a problem hiding this comment.
@lhy1024 Did you see this reply? #9372 (comment)
There was a problem hiding this comment.
You are right. We only need to solve hot read change by store heartbeat.
Signed-off-by: okJiang <[email protected]>
| c.GetHotStat().CheckWriteAsync(checkExpiredTask) | ||
| c.GetHotStat().CheckReadAsync(checkExpiredTask) | ||
| c.GetHotStat().CheckReadAsync(checkLeaderChangeTask) |
There was a problem hiding this comment.
It may need to be checked with write.
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lhy1024 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
needs-1-more-lgtm
|
/retest |
What problem does this PR solve?
Issue Number: Close #9366
What is changed and how does it work?
Check List
Tests
Release note