Skip to content

Conversation

kostasrim
Copy link
Contributor

@kostasrim kostasrim commented Sep 4, 2025

This PR implements the new ReplicaOf algorithm that does not use two phase locking.

How ReplicaOf worked previously:

  1. stop the old replica -> dragonfly is processing read only commands normally
  2. Set the flag to loading -> dragonfly stops processing read only commands because state is loading
  3. create a new replica object
  4. release the mutex
  5. allow the new replica object to do IO by connecting to the new master and exchange the necessary info to actually start the replication flow. -> This may fail yet we prematurely set the flag to loading and stopped the old replica
  6. relock the mutex after the greet, clean up if the context got cancelled (by another replicaof command) or start the main replication fiber to properly start replication.

There are multiple issue with this:

  1. two phase locking, allows other replicaof commands to interleave (not serialized) which complicates state transitions
  2. prematurely setting state to loading even if the connection fails later on after releasing the mutex
  3. prematurely stopped replication even if the connection with the new master fails later on
  4. prematurely stops processing read only commands. Masters might be rotating so we might enter partial sync immediately. We don't need to enter LOADING state for that.

How it works now:

The new algorithm is very simple and has only two steps:

  1. Create a Replica object, Initiate connection to the new master, greet and exchange the necessary info to setup replication. (internally it’s just REPLCONF setting connection members and info). So far there is no update to the state. We merely check if we can establish a connection with the new master and that everything is ok.

  2. If there are no errors, lock the mutex, update the replica_ object and start the MainReplicationFiber. Do not enter LOADING state prematurely. First check if partial sync is available in main replication fiber and if not, then enter LOADING state and do full sync.

Now all ReplicaOfInternal commands are serialized. Cancellation is not affected because the connection that enters the critical section of (b) will cancel/stop the previous one. This happens one after the other in a deterministic manner.

Signed-off-by: kostas <kostas@dragonflydb.io>
@kostasrim kostasrim marked this pull request as ready for review September 5, 2025 07:21
@kostasrim kostasrim requested a review from romange September 5, 2025 07:21
}

void ServerFamily::ReplicaOf(CmdArgList args, const CommandContext& cmd_cntx) {
ReplicaOfInternal(args, cmd_cntx.tx, cmd_cntx.rb, ActionOnConnectionFail::kReturnOnError);
Copy link
Collaborator

@romange romange Sep 7, 2025

Choose a reason for hiding this comment

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

It seems that changes are grouped together nicely. Let's introduce the flag experimental_replicaof_v2 set to true so we could have the fallback to the old variant


auto new_replica = make_shared<Replica>(replicaof_args->host, replicaof_args->port, &service_,
master_replid(), replicaof_args->slot_range);
GenericError ec{};
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
GenericError ec{};
GenericError ec;


if (!ss->is_master) {
CHECK(replica_);
// flip flag before clearing replica_
Copy link
Collaborator

Choose a reason for hiding this comment

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

style: please keep an empty line before comment lines. here and everywhere.

@romange
Copy link
Collaborator

romange commented Sep 7, 2025

What's the functional change now? how it is changed against the previous version?

@kostasrim kostasrim requested a review from romange September 10, 2025 15:14
@kostasrim kostasrim merged commit c9acc94 into main Sep 27, 2025
10 checks passed
@kostasrim kostasrim deleted the kpr5 branch September 27, 2025 13:23
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