Skip to content

Handle some scenarios that we need to cancel ASM#57

Merged
ShooterIT merged 11 commits intotezc:cluster-asmfrom
ShooterIT:asm-delnode
Aug 22, 2025
Merged

Handle some scenarios that we need to cancel ASM#57
ShooterIT merged 11 commits intotezc:cluster-asmfrom
ShooterIT:asm-delnode

Conversation

@ShooterIT
Copy link
Collaborator

@ShooterIT ShooterIT commented Aug 18, 2025 •
edited
Loading

  • CLUSTER FORGET command
  • CLUSTER ADDSLOTS/DELSLOTS command
  • CLUSTER SETSLOTS command
  • CLUSTER FAILOVER command
  • CLIENT PAUSE command
  • server shutdown (SHUTDOWN command or kill)

@tezc
Copy link
Owner

tezc commented Aug 18, 2025

@ShooterIT As you are working on this, I think we should also consider cancelling on FAILOVER. Master will pause writes during failover and if we complete migration during this write pause, source side will try to trim keys which will also try to write to repl log.

.. -> clusterDelKeysInSlot() -> .. -> postExecutionUnitOperations() -> ... -> propagateNow()

It eventually triggers an assert here:
https://github.com/redis/redis/blob/b9d9d4000b0b45b87c7c6fea23ec9fd8fcac107e/src/server.c#L3480

@ShooterIT
Copy link
Collaborator Author

ShooterIT commented Aug 19, 2025

thanks, i missed that, you mean failover command, right?
btw, i encountered the same crash when server shutdown

@tezc
Copy link
Owner

tezc commented Aug 19, 2025

yes, CLUSTER FAILOVER command

@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 92.72727% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (cluster-asm@3c932e3). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/cluster_asm.c 91.11% 4 Missing ⚠️
Additional details and impacted files 
@@              Coverage Diff               @@
##             cluster-asm      #57   +/-   ##
==============================================
  Coverage               ?   69.69%           
==============================================
  Files                  ?      125           
  Lines                  ?    73876           
  Branches               ?        0           
==============================================
  Hits                   ?    51487           
  Misses                 ?    22389           
  Partials               ?        0
Files with missing lines Coverage Δ
src/cluster_legacy.c 80.84% <100.00%> (ø)
src/networking.c 91.33% <100.00%> (ø)
src/server.c 89.40% <100.00%> (ø)
src/cluster_asm.c 88.62% <91.11%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ShooterIT
ShooterIT commented Aug 21, 2025
View reviewed changes
@ShooterIT ShooterIT requested a review from tezc August 21, 2025 13:49
tezc
tezc reviewed Aug 21, 2025
View reviewed changes
tezc
tezc reviewed Aug 21, 2025
View reviewed changes
tezc
tezc reviewed Aug 21, 2025
View reviewed changes
@ShooterIT ShooterIT merged commit e47f72c into tezc:cluster-asm Aug 22, 2025
16 of 18 checks passed
@ShooterIT ShooterIT deleted the asm-delnode branch August 22, 2025 06:08
tezc pushed a commit that referenced this pull request Sep 10, 2025
@ShooterIT @tezc
Handle some scenarios that we need to cancel ASM (#57)
d7ff99e 
- CLUSTER FORGET command
- CLUSTER ADDSLOTS/DELSLOTS command
- CLUSTER SETSLOTS command
- CLUSTER FAILOVER command
- CLIENT PAUSE command
- server shutdown (SHUTDOWN command or kill)
tezc pushed a commit that referenced this pull request Sep 16, 2025
@ShooterIT @tezc
Handle some scenarios that we need to cancel ASM (#57)
48d86dc 
- CLUSTER FORGET command
- CLUSTER ADDSLOTS/DELSLOTS command
- CLUSTER SETSLOTS command
- CLUSTER FAILOVER command
- CLIENT PAUSE command
- server shutdown (SHUTDOWN command or kill)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tezc tezc tezc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ShooterIT @tezc @codecov-commenter