Skip to content

Custom Procedure Replication Propagation #1252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

vazois
Copy link
Collaborator

@vazois vazois commented Jun 12, 2025

This PR fixes #1250

@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 21:43
Copy link
Contributor

@Copilot 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 adds support for replicating custom stored procedures through the AOF and cluster sessions, and validates that behavior with a new integration test.

  • Extend RespServerSession and AofProcessor to accept an optional IClusterProvider and wire it through session creation.
  • Update AofProcessor.ReplayOp to replay stored procedures with read/write session toggling.
  • Add a ClusterReplicationStoredProc test in the cluster tests and include the RateLimiterTxn helper class.

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 Add ClusterReplicationStoredProc test to verify stored-procedure replication
test/Garnet.test.cluster/Garnet.test.cluster.csproj Include RateLimiterTxn.cs in the test project
libs/server/Resp/RespServerSession.cs Add clusterProvider parameter to constructor and use it to create clusterSession
libs/server/Databases/SingleDatabaseManager.cs Use named logger: parameter when instantiating AofProcessor
libs/server/Databases/MultiDatabaseManager.cs Use named logger: parameter when instantiating AofProcessor
libs/server/AOF/AofProcessor.cs Introduce clusterProvider ctor parameter, nest RunStoredProc inside ReplayOp, rename a flag
libs/cluster/Server/Replication/ReplicationManager.cs Pass clusterProvider into AofProcessor instantiation
Comments suppressed due to low confidence (2)

test/Garnet.test.cluster/ReplicationTests/ClusterReplicationBaseTests.cs:1186

  • [nitpick] Local variable replica_count uses snake_case. Rename to replicaCount to follow C# naming conventions.
var replica_count = 1;// Per primary

libs/server/Resp/RespServerSession.cs:226

  • The XML documentation is missing a <param> entry describing the clusterProvider parameter. Please add a description.
IClusterProvider clusterProvider = null)

@vazois vazois marked this pull request as draft June 12, 2025 21:52
@vazois vazois force-pushed the vazois/replication-proc branch from e620ec6 to 030e15b Compare June 12, 2025 21:54
@vazois vazois marked this pull request as ready for review June 12, 2025 21:54
@TalZaccai TalZaccai requested review from badrishc and TalZaccai June 18, 2025 18:10
@vazois vazois force-pushed the vazois/replication-proc branch from 146225f to 92615af Compare June 20, 2025 16:57
@vazois vazois force-pushed the vazois/replication-proc branch from 92615af to db68db1 Compare June 20, 2025 18:19
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.

Replication does not work for Transactions
2 participants