Skip to content

Add configurable backup snapshot fetching from primary#7695

Merged
eddyashton merged 26 commits into
mainfrom
copilot/add-snapshot-fetching-feature
Mar 11, 2026
Merged

Add configurable backup snapshot fetching from primary#7695
eddyashton merged 26 commits into
mainfrom
copilot/add-snapshot-fetching-feature

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 3, 2026

  • Add backup snapshot fetch feature with BackupSnapshotFetch task, snapshot evidence hook, config plumbing, and e2e test
  • Remove max_size from backup snapshot fetch configuration
  • Add target_rpc_interface config to backup snapshot fetch
  • Fix C++ formatting (clang-format) in configuration.h and node_state.h

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI and others added 3 commits March 3, 2026 11:54
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copilot AI changed the title [WIP] Add configurable snapshot fetching for backup nodes Add configurable backup snapshot fetching from primary Mar 3, 2026
@achamayou
Copy link
Copy Markdown
Member

@copilot drop the max_size constraint and configuration, it is not necessary here.

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 3, 2026

@copilot drop the max_size constraint and configuration, it is not necessary here.

Removed max_size from the BackupFetch config struct, JSON serialization, config schema, and CHANGELOG. The underlying fetch_from_peer call now uses std::numeric_limits<size_t>::max() instead. Done in 3b9c163.

@achamayou
Copy link
Copy Markdown
Member

@copilot ok actually we need one more thing in the configuration here: the name of the rpc_interface to use on the target node to download the snapshot. That has to be used when grabbing the network details of the primary from the KK to attempt the fetch from it in the BackupSnapshotFetch task. We can't just take the first interface, it may not have the SnapshotRead feature enabled on it.

Copilot AI and others added 2 commits March 4, 2026 19:59
The BackupSnapshotFetch task now looks up the primary's RPC interface
by name (configured via target_rpc_interface) instead of using the
first available interface. This ensures the correct interface with
SnapshotRead enabled is used for downloading snapshots.

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 4, 2026

@copilot ok actually we need one more thing in the configuration here: the name of the rpc_interface to use on the target node to download the snapshot. That has to be used when grabbi...

Added target_rpc_interface field to BackupFetch config (default: "primary_rpc_interface"). The BackupSnapshotFetch task now looks up the primary's interface by this configured name via rpc_interfaces.find() instead of using the first available interface. Done in 388c734.

@achamayou
Copy link
Copy Markdown
Member

@copilot run ./scripts/ci-checks.sh -f to format the code

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 5, 2026

@copilot run ./scripts/ci-checks.sh -f to format the code

Fixed C++ formatting in configuration.h (multi-line macro args) and node_state.h (line length). Python files were already clean. Done in df810b9.

@achamayou achamayou marked this pull request as ready for review March 6, 2026 22:51
@achamayou achamayou requested a review from a team as a code owner March 6, 2026 22:51
Copilot AI review requested due to automatic review settings March 6, 2026 22:51
Comment thread CHANGELOG.md Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an automatic “backup snapshot fetch” path where backup nodes can download snapshots from the primary after snapshot evidence is committed, with configuration surfaced through startup config parsing, host config schema, and e2e coverage.

Changes:

  • Extend snapshot fetch client to support since query parameter and plumb it into a new backup snapshot fetch task.
  • Add snapshots.backup_fetch configuration plumbing (startup config JSON parsing + host config schema + test config template).
  • Add an e2e test runner for snapshot downloading and update devcontainer runtime settings.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/schema.py Registers a new “download-snapshot” test entry invoking the new e2e scenario.
tests/infra/remote.py Adds test-infra plumbing to render snapshots.backup_fetch into node configs.
tests/infra/interfaces.py Adds accessor for the file-serving RPC interface (used for snapshot download in tests).
tests/e2e_operations.py Adds e2e coverage for backups fetching snapshots (and a max-size negative test).
tests/config.jinja Emits snapshots.backup_fetch config block when enabled in tests.
src/snapshots/fetch.h Adds optional since_seqno support and updates logging accordingly.
src/node/node_state.h Introduces BackupSnapshotFetch task + snapshot evidence commit hook to trigger fetching on backups.
src/common/configuration.h Adds JSON parsing declarations for CCFConfig::Snapshots::BackupFetch and backup_fetch field.
include/ccf/node/startup_config.h Adds Snapshots::BackupFetch config struct (enabled/max_attempts/retry_interval/target_rpc_interface/max_size).
doc/host_config_schema/cchost_config.json Documents snapshots.backup_fetch section in the host config schema.
CHANGELOG.md Documents the new backup snapshot fetch feature.
.devcontainer/devcontainer.json Enables privileged devcontainer runs.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread tests/e2e_operations.py
Comment thread CHANGELOG.md
Comment thread include/ccf/node/startup_config.h Outdated
Comment thread src/node/node_state.h
Comment thread tests/e2e_operations.py Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread include/ccf/node/startup_config.h
Comment thread src/node/node_state.h
Comment thread include/ccf/node/startup_config.h Outdated
Comment thread src/node/node_state.h
@eddyashton eddyashton merged commit 93db435 into main Mar 11, 2026
17 checks passed
@eddyashton eddyashton deleted the copilot/add-snapshot-fetching-feature branch March 11, 2026 14:46
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.

5 participants