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

Feature/fleet mode rebase #6641

Merged
merged 5 commits into from May 22, 2024

Conversation

garyschulte
Copy link
Contributor

@garyschulte garyschulte commented Feb 28, 2024

PR description

rebase onto main of the services refactor for fleet mode, see original #6003

Fixed Issue(s)

@garyschulte garyschulte marked this pull request as ready for review March 1, 2024 01:09
@garyschulte garyschulte force-pushed the feature/fleet-mode-rebase branch 2 times, most recently from b6689c9 to 87104d6 Compare March 1, 2024 01:16
Copy link
Contributor

@matkt matkt left a comment

Choose a reason for hiding this comment

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

did we tried follower+leader with this last commit ?

@matkt
Copy link
Contributor

matkt commented Mar 1, 2024

@jframe could you check the block pruning modification we are doing in this PR to be sure it's correct. normally we did tests but I prefer to have your feedback

@garyschulte
Copy link
Contributor Author

garyschulte commented Mar 1, 2024

@jframe could you check the block pruning modification we are doing in this PR to be sure it's correct. normally we did tests but I prefer to have your feedback

I inadvertentlty tested this feature on the captain of a captain-follower sepolia pair, and block pruning is definitely working.

did we tried follower+leader with this last commit ?

I am resyncing the follower now to ensure it works correctly, since it failed to find the pruned captain blocks on the prior test 😂

@garyschulte
Copy link
Contributor Author

did we tried follower+leader with this last commit ?

Yes, confirmed the follower syncs and follows with only a leader.

Regarding the pruner on the follower, I see it configured, but I am not seeing any executions of the pruner triggered.

@jframe
Copy link
Contributor

jframe commented Mar 4, 2024

@jframe could you check the block pruning modification we are doing in this PR to be sure it's correct. normally we did tests but I prefer to have your feedback

Is the changes in the ChainDataPruner to only start pruning after the initial sync? Think it's better not to delay this until after the initial sync to keep the storage used small. Ideally, you would have Besu configured with --Xcheckpoint-post-merge-enabled so there isn't anything to prune anyway. Probably need to think more about how these and sync all interact.

@garyschulte garyschulte force-pushed the feature/fleet-mode-rebase branch 3 times, most recently from e4c8b4a to 9a279ec Compare March 9, 2024 00:53
@garyschulte garyschulte force-pushed the feature/fleet-mode-rebase branch 2 times, most recently from 465042a to 4fb1a3a Compare April 16, 2024 20:11
@garyschulte garyschulte force-pushed the feature/fleet-mode-rebase branch 2 times, most recently from 3f4e197 to 8836bfb Compare April 22, 2024 19:26
Copy link
Contributor

@matkt matkt left a comment

Choose a reason for hiding this comment

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

if we had time to do a quick test with the fleet plugin I think we can merge this PR

Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte enabled auto-merge (squash) May 22, 2024 16:37
@garyschulte garyschulte merged commit ebb8830 into hyperledger:main May 22, 2024
37 checks passed
@non-fungible-nelson
Copy link
Contributor

🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢🚢

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.

None yet

4 participants