Add configurable backup snapshot fetching from primary#7695
Add configurable backup snapshot fetching from primary#7695
Conversation
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 drop the max_size constraint and configuration, it is not necessary here. |
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
|
@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. |
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>
Added |
|
@copilot run ./scripts/ci-checks.sh -f to format the code |
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
There was a problem hiding this comment.
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
sincequery parameter and plumb it into a new backup snapshot fetch task. - Add
snapshots.backup_fetchconfiguration 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.
|
|
||
| ### Added | ||
|
|
||
| - Backup nodes can now be configured to automatically fetch snapshots from the primary when snapshot evidence is detected. This is controlled by the `snapshots.backup_fetch` configuration section, with `enabled`, `max_attempts`, `retry_interval`, `max_size` and `target_rpc_interface` options. Note that the target RPC interface selected must have the `SnapshotRead` operator feature enabled. |
There was a problem hiding this comment.
The changelog entry lists the new snapshots.backup_fetch options but omits max_size, which is present in the config struct/schema/template in this PR. Please either document max_size here as well, or remove max_size support from the implementation/schema so the changelog matches actual behaviour.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
🔒 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.