some improvements for slot migration#55
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## cluster-asm #55 +/- ##
==============================================
Coverage ? 69.63%
==============================================
Files ? 125
Lines ? 73777
Branches ? 0
==============================================
Hits ? 51376
Misses ? 22401
Partials ? 0
🚀 New features to boost your workflow:
|
| myself->flags |= CLUSTER_NODE_SLAVE; | ||
| clusterCloseAllSlots(); | ||
| /* Cancel all ASM tasks when switching into slave */ | ||
| clusterAsmCancel(NULL, "switching to replica"); |
There was a problem hiding this comment.
Hi @tezc not sure if the cluster plugin has this function to allow us to cancel asm tasks
if not, we can check the change of role in asmBeforeSleep, or other places, but it looks like ugly.
There was a problem hiding this comment.
I see we send ASM_EVENT_FAILED event but we can also send ASM_EVENT_CANCELLED to indicate the task is cancelled. I can handle it later.
There was a problem hiding this comment.
no, i mean do we have a similar clusterSetMaster function, switching a master to replica
There was a problem hiding this comment.
oh I see. I'll check with Josh later. Plugin may cancel itself but perhaps it is always a good idea that redis should be notified whenever there is a config change like this.
There was a problem hiding this comment.
plugin should cancel asm in such cases:
- change slots directly
- switch master to slave
- delete cluster node (i think i will fix in next PR)
There was a problem hiding this comment.
I remember we talked about this before, plugin would call us to notify on config change but not yet implemented. Perhaps, plugin may cancel externally but we can also internally cancel it on this notification (just to avoid ugly scenarios in case there is a bug in plugin)
tezc
left a comment
There was a problem hiding this comment.
thanks, this PR is going to cleanup lots of mess
- flush-like command will cancel slot migration task - switching to replica from master will cancel slot migration task - resolve the source node when starting importing tasks - hide rdb/main channel client in info replication - in client list, migrating client is marked as m, importing client is marked as i - show asm_migrating_buffer and asm_importing_buffer in info memory - reject cluster setslot if there is an active slot migration, and cancel tasks if slot topology is changed
- flush-like command will cancel slot migration task - switching to replica from master will cancel slot migration task - resolve the source node when starting importing tasks - hide rdb/main channel client in info replication - in client list, migrating client is marked as m, importing client is marked as i - show asm_migrating_buffer and asm_importing_buffer in info memory - reject cluster setslot if there is an active slot migration, and cancel tasks if slot topology is changed
info replicationclient list, migrating client is marked asm, importing client is marked asiasm_migrating_bufferandasm_importing_bufferininfo memory