Skip to content
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

Add more RAFT tests #4626

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add more RAFT tests #4626

wants to merge 1 commit into from

Conversation

mprimi
Copy link
Contributor

@mprimi mprimi commented Oct 4, 2023

Add 'Chain of Blocks' tests for Raft/NRG

Chain of Blocks is a simple state machine on top of Raft/NRG.
Values delivered through Raft are hashed allowing for easy verification of replicas divergence.

  • Add one happy-path test
  • Add one test for recovery/catchup
  • Add a 'Long' randomized test

Signed-off-by: [email protected]

@mprimi mprimi force-pushed the marco/raft-chain-test branch from 76267e5 to 6789d2d Compare December 7, 2023 20:11
@mprimi mprimi force-pushed the marco/raft-chain-test branch from 6789d2d to e92592e Compare January 3, 2024 23:13
@mprimi mprimi force-pushed the marco/raft-chain-test branch from e2744a4 to 7950364 Compare January 19, 2024 20:48
@mprimi mprimi force-pushed the marco/raft-chain-test branch 3 times, most recently from a73cbc3 to 7eb306e Compare December 19, 2024 09:28
@mprimi mprimi marked this pull request as ready for review December 19, 2024 16:24
@mprimi mprimi requested a review from a team as a code owner December 19, 2024 16:24
Chain of Blocks is a simple state machine on top of Raft/NRG.
Values delivered through Raft are hashed allowing for easy verification
of replicas divergence.

 - Add one happy-path test
 - Add one test for recovery/catchup
 - Add a 'Long' randomized test
@mprimi mprimi force-pushed the marco/raft-chain-test branch from 7eb306e to fc3553a Compare January 2, 2025 21:33
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

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

One comment, otherwise LGTM

Comment on lines +154 to +159
sm.Lock()
defer sm.Unlock()
sm.n.Stop()
sm.wg.Wait()

// Clear state, on restart it will be recovered from snapshot or peers
Copy link
Member

Choose a reason for hiding this comment

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

When running locally it seems the test can lock up and not do anything/exceed the test duration.

Should be fixed by not holding the lock while waiting for Stop/Wait to complete The same logic is used in stateAdder.stop().

Suggested change
sm.Lock()
defer sm.Unlock()
sm.n.Stop()
sm.wg.Wait()
// Clear state, on restart it will be recovered from snapshot or peers
n, wg := sm.node(), sm.waitGroup()
n.Stop()
n.WaitForStop()
wg.Wait()
// Clear state, on restart it will be recovered from snapshot or peers
sm.Lock()
defer sm.Unlock()

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