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

Checkpoint integration test #2343

Merged
merged 6 commits into from
Jun 6, 2022
Merged

Checkpoint integration test #2343

merged 6 commits into from
Jun 6, 2022

Conversation

asonnino
Copy link
Contributor

Test the integration of the checkpointing logic with consensus.

@asonnino asonnino self-assigned this May 31, 2022
@asonnino asonnino requested review from bmwill, gdanezis, huitseeker and lxfind and removed request for huitseeker May 31, 2022 18:12
Copy link
Collaborator

@gdanezis gdanezis left a comment

Choose a reason for hiding this comment

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

Ok, we are really close now -- nice work. Now we can test end-to-end!

crates/sui-core/src/checkpoints/mod.rs Show resolved Hide resolved
@@ -147,6 +154,7 @@ impl AuthorityServer {
pub struct ValidatorService {
state: Arc<AuthorityState>,
consensus_adapter: ConsensusAdapter,
_checkpoint_consensus_handle: Option<JoinHandle<()>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, this is not going to be so simple, since we will need to restart it upon reconfiguration? We will work out the right structure down the line.

@@ -295,7 +302,7 @@ pub struct CheckpointConsensusAdapter {
/// Receive new checkpoint fragments to sequence.
rx_checkpoint_consensus_adapter: Receiver<CheckpointFragment>,
/// A pointer to the checkpoints local store.
checkpoint_locals: ArcSwapOption<CheckpointLocals>,
checkpoint_locals: Arc<CheckpointLocals>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for this! Indeed we do not need a swap here.

pub fn spawn(mut instance: Self) -> JoinHandle<()> {
tokio::spawn(async move { instance.run().await })
pub fn spawn(mut self) -> JoinHandle<()> {
tokio::spawn(async move { self.run().await })
Copy link
Collaborator

Choose a reason for hiding this comment

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

We will have to track its lifetime and restart/halt propertly.

@asonnino asonnino enabled auto-merge (squash) June 6, 2022 16:27
@asonnino asonnino merged commit 893b4b4 into main Jun 6, 2022
@asonnino asonnino deleted the checkpoint-integration-test branch June 6, 2022 16:43
stella3d pushed a commit that referenced this pull request Jun 7, 2022
checkpoint-consensus integration test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants