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

snapshot: fix incremental snapshot loading #3983

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

Conversation

topointon-jump
Copy link
Contributor

@topointon-jump topointon-jump commented Jan 22, 2025

In the case where we have an incremental snapshot, the status cache and the slot context were first being initialized from the incremental snapshot preflight manifest load, and then again when we loaded the full snapshot manifest.

This meant that the validators slot number is set first to the incremental snapshots slot number, then jumps back to the full snapshots slot number. This causes repair to get stuck in a loop.

Therefore, if we have an incremental snapshot this PR skips initializing the status cash and the slot context when we load in the full snapshot - because these will have already been initialized by the incremental snapshot preflight manifest load.

Comment on lines +146 to +135
int do_restore_manifest,
int do_restore_status_cache ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest using flags here. It's easier to extend in the future and the call sites become more readable.

fd_snapshot_load_manifest_and_status_cache(ctx,1,1) becomes fd_snapshot_load_manifest_and_status_cache(ctx, FD_SNAPSHOT_RESTORE_MANIFEST|FD_SNAPSHOT_RESTORE_STATUS_CACHE)

Copy link
Contributor

Choose a reason for hiding this comment

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

(In case Agave prepends yet another data structure)

@topointon-jump topointon-jump force-pushed the topointon/fix-snapshot-load branch from 1f496e1 to 4b90878 Compare January 22, 2025 03:39
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