Skip to content

Disable replication on Close - #492

Open
samliok wants to merge 2 commits into
mainfrom
replication-close
Open

Disable replication on Close#492
samliok wants to merge 2 commits into
mainfrom
replication-close

Conversation

@samliok

@samliok samliok commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Making this change means that if we call Close on replication state, we don't need to worry about calling AdvanceTime or frankly any of the other methods on it.

@yacovm

yacovm commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

I don't understand what goes wrong without this change.

Is there any scenario that can be demonstrated that fails or gets stuck without this change?

@samliok

samliok commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

I don't understand what goes wrong without this change.

Is there any scenario that can be demonstrated that fails or gets stuck without this change?

I was concerned that our shutdown for non-validator and validator could break depending on the ordering of the Close calls. Haven't written a test to show it breaking, but the potential issue is what if we close replication and then the epoch/non-validator is still processing replication messages. It will be trying to update the state of a closed struct, so maybe this would cause an ungraceful shutdown.

func (n *NonValidator) Stop() {
	n.Logger.Info("Shutting down non-validator", zap.Stringer("ID", n.ID))
	n.cancelCtx()
	n.sequenceReplicator.Close()
	n.verifier.Close()
}
func (e *Epoch) Stop() {
	e.Logger.Info("Shutting down node")
	e.finishFn()
	e.monitor.Close()
	e.blockVerificationScheduler.Close()
	e.buildBlockScheduler.Close()
	e.timeoutHandler.Close()
	e.replicationState.Close()
}

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