Full Replication #274

samliok · 2025-10-09T22:56:58Z

Summary

This PR finalizes the replication scheme by clearly distinguishing between replicating rounds and replicating sequences. It also refactors related components for clarity and reliability. Overall, the replication logic is much more straight-forward and simple which will hopefully lead to less bugs(a few small ones I have found while making the pr)

Replication Overview

In Simplex, when a node receives a notarization, emptyNotarization, or finalization for a round or sequence ahead of its current state, it indicates the node is behind. The process of catching up(retrieving missing rounds or sequences) is called Replication.

The replicationState struct manages this process. Replication is triggered when:

A notarization or emptyNotarization for a higher round is received, or
A finalization for a future sequence arrives.
It can also be triggered when a lagging node sends an empty vote for an older round. In this case, the normal node will send its highest notarization and finalization to potentially trigger replication for the lagging node.

The replicationState tracks missing rounds and sequences, resends requests as needed, and removes completed items. When a QuorumRound (notarization, empty notarization, or finalization) is received, it’s added to the relevant state. Finalizations always supersede earlier quorum rounds, allowing us to prune older states from both the rounds and sequence trackers.

processReplicationState advances rounds or sequences by checking for available quorum rounds. If the next sequence is complete, it’s committed; otherwise, the function checks for the next round. Empty notarizations can sometimes be inferred to advance rounds via maybeAdvanceRoundFromEmptyNotarizations.

Key Changes

Introduced replicator struct, which:
- Stores received QuorumRounds keyed by rounds or sequenc es.
- Limits requests to a maximum of maxRoundWindow ahead of the current position.
- Manages timeouts for missing quorum rounds.
Simplified TimeoutManager:
- As long as tasks exist in taskMap, the TimeoutHandler periodically sends them to the TaskRunner every runInterval.
- This greatly simplifies task management and scheduling logic.

samliok · 2025-10-10T19:53:20Z

epoch.go

 func (e *Epoch) maybeSendNotarizationOrFinalization(to NodeID, round uint64) {
 	r, ok := e.rounds[round]

 	if !ok {


if we have an empty notarization and a notarization for the same round, which one should we prioritze sending?

samliok · 2025-10-10T19:57:43Z

epoch.go

-			e.Logger.Debug("Received invalid quorum round", zap.Uint64("seq", data.GetSequence()), zap.Stringer("from", from))
+		// TODO: because empty notarizations can be for long periods, we may want to allow messages that are finalized to
+		// be stored even if they are > maxRoundWindow rounds ahead. For now we allow only the nextSeqToCommit but we might want to
+		// allow a few more seqs ahead.


small bug i caught with our old replication logic

samliok · 2025-10-10T19:58:34Z

epoch.go

 	// make sure the latest round is well formed
 	if err := latestRound.IsWellFormed(); err != nil {
-		e.Logger.Debug("Received invalid latest round", zap.Error(err))
-		return err


garbage collection is now managed by the replicationState

yacovm · 2025-10-10T20:29:12Z

I made this test in the past, maybe it can be useful.

yacovm · 2025-10-13T15:27:00Z

processReplicationState advances epochs by checking for available quorum rounds.

advances rounds/sequences?

replication_test.go

Signed-off-by: Sam Liokumovich <[email protected]>

yacovm

Made a pass, will make another pass later.

testutil/comm.go

yacovm · 2025-10-16T14:21:41Z

testutil/node.go

+
+// TimeoutOnRound advances time until the node times out of the given round.
+func (t *TestNode) TimeoutOnRound(round uint64) {
+	startTime := t.E.StartTime


I think it makes sense to have a time as part of the TestNode struct, otherwise you can only use this method properly once. When this method returns, the start time of the node is out of sync with the E.StartTime and you will start from the old start time the next time you invoke this method. Instead I think we should store the current time in the TestNode and when you update the time you also save it in the TestNode. This way, next time you invoke this method, you will fetch the updated start time.

u added a very helpful currentTime variable as part of testNode so I'll use that 🎉

yacovm · 2025-10-16T14:23:00Z

testutil/node.go

+			return
+		}
+
+		time.Sleep(100 * time.Millisecond)


why do we sleep here?

just to give a little throttle for the for loop and allow messages to be processed by the node. reduced it to 50ms bc i think 100 was too much.

api.go

epoch_failover_test.go

yacovm · 2025-10-16T21:56:04Z

replicator.go

+// it limits the amount of outstanding requests to be at most [maxRoundWindow] ahead of [base] which is
+// either nextSeqToCommit or currentRound depending on if we are replicating sequences or rounds.
+func (r *replicator) maybeSendMoreReplicationRequests(observed *signerRoundOrSeq, base uint64) {
+	val := observed.value()


i don't think we should be having any variable called "val". It's only slightly more generic than "variable".

yacovm · 2025-10-16T21:56:49Z

replicator.go

+// maybeSendMoreReplicationRequests checks if we need to send more replication requests given an observed round or sequence.
+// it limits the amount of outstanding requests to be at most [maxRoundWindow] ahead of [base] which is
+// either nextSeqToCommit or currentRound depending on if we are replicating sequences or rounds.
+func (r *replicator) maybeSendMoreReplicationRequests(observed *signerRoundOrSeq, base uint64) {


why call this base and not nextSeqToCommit ?

the naming is a bit weird and im very open to suggestions. specifically regarding this and .value(). the reason its not nextSeq to commit is because it can be the nextSeqToCommit or the current round.

potentionallyHighestroundOrSequence ?

yacovm · 2025-10-16T22:00:34Z

replicator.go

+	}
+
+	startSeq := math.Max(float64(base), float64(r.highestRequested))
+	// we limit the number of outstanding requests to be at most maxRoundWindow ahead of nextSeqToCommit


we limit the number of outstanding requests to be at most maxRoundWindow ahead of nextSeqToCommit

why? doesn't that mean we will only replicate up to max round window?

yes, but we only replicate to maxRoundWindow ahead at a single time. When the state advances, we check if we need to send more. This way if we are behind by a gazilion blocks our memory doesn't get overwhelmed

I get the memory concern, I am just wondering if it makes sense to use the same config parameter for both the standard execution flow and the replication one.

I guess we can put them the same for now, maybe revisit later.

yacovm · 2025-10-16T22:03:34Z

replicator.go

+// sendReplicationRequests sends requests for missing sequences for the
+// range of sequences [start, end] <- inclusive. It does so by splitting the
+// range of sequences equally amount the nodes that have signed [highestSequenceObserved].
+func (r *replicator) sendReplicationRequests(start uint64, end uint64) {


This was just moved over from replication.go right?

yacovm · 2025-10-16T22:13:29Z

replicator.go

+// sendRequestToNode requests the sequences [start, end] from nodes[index].
+// In case the nodes[index] does not respond, we create a timeout that will
+// re-send the request.
+func (r *replicator) sendRequestToNode(start uint64, end uint64, nodes []NodeID, index int) {


when we call resendReplicationRequests we call sendRequestToNode with seqs.Start, seqs.End where we call AddTask() on the index.

Isn't this a repetitive re-addition of the sequences to the map? Why do we need to do that again and again?

it would be a no-op though since the task is already in the scheduler. I can add a flag to sendRequestToNode such as shouldAddTimeout and only add the timeout if set.

yacovm · 2025-10-17T19:47:12Z

und": 0, "size": 18}
2025-10-17T19:32:40.4304984Z panic: test timed out after 10m0s
2025-10-17T19:32:40.4305063Z    running tests:
2025-10-17T19:32:40.4305236Z            TestReplicationRequestWithoutFinalization (8m58s)
2025-10-17T19:32:40.4305246Z 
2025-10-17T19:32:40.4305329Z goroutine 4950 [running]:
2025-10-17T19:32:40.4305420Z testing.(*M).startAlarm.func1()
2025-10-17T19:32:40.4305637Z    /opt/hostedtoolcache/go/1.23.12/x64/src/testing/testing.go:2373 +0x265
2025-10-17T19:32:40.4305715Z created by time.goFunc

samliok · 2025-10-17T20:19:18Z

replication_timeout_test.go

 		msg.VerifiedReplicationResponse.Data = newData
-		c.replicationResponses <- struct{}{}
+		select {
+		case c.replicationResponses <- struct{}{}:


this is not ideal, but was causing a flake because now we send extra replication responses. Once if we receive a notarization, and another if we receive finalizations.

yacovm

Will continue to review later, here are some more comments.

yacovm · 2025-10-19T21:16:09Z

epoch.go

+		return nil
+	}
+
+	// for future rounds


what about future rounds?

yacovm · 2025-10-19T21:22:18Z

replicator.go

+// maybeSendMoreReplicationRequests checks if we need to send more replication requests given an observed round or sequence.
+// it limits the amount of outstanding requests to be at most [maxRoundWindow] ahead of [base] which is
+// either nextSeqToCommit or currentRound depending on if we are replicating sequences or rounds.
+func (r *replicator) maybeSendMoreReplicationRequests(observed *signerRoundOrSeq, base uint64) {


potentionallyHighestroundOrSequence ?

yacovm · 2025-10-19T21:30:33Z

replicator.go

+	val := observed.value()
+
+	// we've observed something we've already requested
+	if r.highestRequested >= val && r.highestObserved != nil {


I am really concerned that we're going to compare apples to oranges if we compare a highest observed sequence to a round, and vice versa.

r.highestObserved can be either a round or a sequence.

Can this not cause an incorrect return?

yacovm · 2025-10-19T21:31:58Z

replicator.go

+	}
+
+	startSeq := math.Max(float64(base), float64(r.highestRequested))
+	// we limit the number of outstanding requests to be at most maxRoundWindow ahead of nextSeqToCommit


I get the memory concern, I am just wondering if it makes sense to use the same config parameter for both the standard execution flow and the replication one.

I guess we can put them the same for now, maybe revisit later.

yacovm · 2025-10-19T21:37:25Z

replicator.go

+		r.highestObserved = observed
+	}
+
+	startSeq := math.Max(float64(base), float64(r.highestRequested))


I don't understand - if highestRequested is a round, how can we treat it as a sequence?

Can we perhaps maintain separate highestObservedX for rounds and sequences? I think it would make things more safer.

yacovm · 2025-10-19T22:09:28Z

epoch.go

-func (e *Epoch) processLatestRoundReceived(latestRound *QuorumRound, from NodeID) error {
+// processQuorumRound processes a quorum round received from another node.
+// It verifies the quorum round and stores it in the replication state if valid.
+func (e *Epoch) processQuorumRound(latestRound *QuorumRound, from NodeID) error {


Not related to this PR, but thought I should mention it.

If we replicate a notarized block $b_i$ that was only notarized by the node that sent us the block and all other nodes notarized the empty round, the next notarized block we receive after that $b_{i+1}$, can be a block building on top of the parent of $b_i$.

Conversely, we can replicate an empty notarization while everyone else but the sender node has a regular notarization for the same round. Then we might get stuck, won't we?

samliok commented Oct 10, 2025

View reviewed changes

samliok commented Oct 13, 2025

View reviewed changes

replication_test.go Show resolved Hide resolved

samliok marked this pull request as ready for review October 13, 2025 21:51

samliok force-pushed the full-replication branch from 0f3cf5b to d443b2d Compare October 13, 2025 21:52

This was linked to issues Oct 15, 2025

Vote for Notarized Blocks during Replication #247

Open

Allow for full replication #254

Open

add replication for rounds and sequences

00bbc72

samliok force-pushed the full-replication branch from d443b2d to 00bbc72 Compare October 15, 2025 18:26

samliok linked an issue Oct 16, 2025 that may be closed by this pull request

Replicate empty rounds that have no notarization / finalization with higher rounds #238

Open

samliok and others added 2 commits October 16, 2025 15:56

Merge branch 'main' into full-replication

9e08e25

Signed-off-by: Sam Liokumovich <[email protected]>

lint

4b6a7c1

yacovm reviewed Oct 16, 2025

View reviewed changes

review comments

134e1ed

fix flake

3f8a366

samliok commented Oct 17, 2025

View reviewed changes

yacovm reviewed Oct 19, 2025

View reviewed changes

Full Replication #274

Are you sure you want to change the base?

Full Replication #274

Uh oh!

Conversation

samliok commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Replication Overview

Key Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yacovm commented Oct 10, 2025

Uh oh!

yacovm commented Oct 13, 2025

Uh oh!

Uh oh!

yacovm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yacovm commented Oct 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yacovm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

samliok commented Oct 9, 2025 •

edited

Loading