Skip to content

Fix Stored Proc Replay#1583

Open
vazois wants to merge 2 commits intomainfrom
vazois/fix-stored-proc-replay
Open

Fix Stored Proc Replay#1583
vazois wants to merge 2 commits intomainfrom
vazois/fix-stored-proc-replay

Conversation

@vazois
Copy link
Contributor

@vazois vazois commented Feb 28, 2026

This PR resolves an issue with slot verification during custom transaction procedure replay.
The replica does not need to re-verify slots because verification is handled by the primary.

Any write that fails verification on the primary is never written to the AOF and therefore cannot reach the replica.
Note: The same invariant applies during migration, where the primary transfers slots and their associated keys to another primary.

Copilot AI review requested due to automatic review settings February 28, 2026 00:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes custom transaction procedure replay in cluster replication by skipping cluster slot ownership verification during AOF replay (replica-side), relying on the invariant that the primary only writes verifiable operations to the AOF.

Changes:

  • Add an isReplaying path for stored procedure execution during AOF replay and propagate it through the replay coordinator/session/transaction manager.
  • Skip slot ownership verification (VerifyKeyOwnership) when a transaction procedure is running under replay.
  • Update cluster replication tests to cover stored proc replication with both object-store and main-store transaction procs, and adjust slot-verification error handling/logging.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Garnet.test.cluster/ReplicationTests/ClusterReplicationBaseTests.cs Expands stored-proc replication test matrix and registers either an object-store or main-store transaction proc; refactors replication attach logic.
libs/server/Transaction/TxnKeyManager.cs Skips cluster slot verification when IsReplaying is true.
libs/server/Transaction/TransactionManager.cs Introduces replay flag plumbing (isReplaying/IsReplaying) for transaction procs and uses it to adjust finalize behavior.
libs/server/Custom/CustomRespCommands.cs Updates stored-proc execution entry point to pass replay intent to the transaction manager.
libs/server/AOF/ReplayCoordinator/AofReplayCoordinator.cs Marks stored proc execution during AOF replay with isReplaying: true.
libs/cluster/Session/SlotVerification/RespClusterIterativeSlotVerify.cs Avoids emitting slot verification responses when OK and adds logging on verification failure.
libs/cluster/Session/SlotVerification/ClusterSlotVerificationResult.cs Adds ToString() to improve slot verification result logging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants