feat: Separate location for ancillary for cardano database v1 #2380

Alenar · 2025-03-19T13:36:41Z

Content

This PR upgrade cardano database v1 snapshots, instead of producing one archive that contains the whole database, two archives are produced:

the main archive that contains all completed immutable files, identified using the same field as before in the artifact (locations and size).
an ancillary archive that contains the last ledger and the uncompleted immutable trio, identified with two new optional fields (ancillary_locations and ancillary_size).

No breaking changes are introduced

for a client that is not updated: it would still be able to validate a restored database, but the ancillary files won't be downloaded leading to a slow start of the restored node.
for updated client: no new options are added on the public api, meaning that the download of the ancillary files is seamless to users and would produce the same result as when there where only one archive.

Details

open-api & mithril-common

Define and add new optional ancillary_locations and ancillary_size fields to Snapshot artifact and messages
Snapshot: removed new ctor, the additions of the new fields triggered the "more than 7 parameters" clippy lint, and since it was not used that much it was more easy to just construct the structure directly than introduce a sub-structure to solve the issue.
DummyCardanoDb: store the path to the created files in the ledger directory

mithril-aggregator

CardanoImmutableFilesFullArtifactBuilder: separate build and upload for completed immutable files and ancillary
LocalSnapshotUploader: removed, its role was to copy the file to its storage location (something also covered by the LocalUploader) and to compute the location uri, but that uri was targeting the /artifact/snapshot/{digest}/download route which could not be easily reused for ancillary download. Instead a LocalUploader is used meaning that, when local upload is set, the locations in the artifact are now /snapshot_download/{filename}.
snapshotter:
- add snapshot_all_completed_immutables
- remove snapshot_all since they are no usage anymore

mithril-client

snapshot_client: updated to also download the ancillary archive if a location is set in ancillary_locations.
feedback: add three new events for cardano db v1 ancillary archive download (SnapshotAncillaryDownloadStarted, SnapshotAncillaryDownloadProgress, and SnapshotAncillaryDownloadCompleted).
refactor integration tests:
- promote the fake.rs as a directory and split its component so there's one file for each integration tests
- put the utility functions to create fake snapshots archive into a snapshot_archives.rs module so they can easily be reused in multiple scenario.

mithril-client-cli

update cardano-db-v1 disk space requirement check so the ancillary archive size is taken into account
update feedback mechanism to add a progress bar when cardano-db-v1 ancillary file is downloaded
fixed dev.json configuration file: the genesis_verification_key was not the one used in mithril-end-to-end

example-cardano-database (v1)

update feedback mechanism to add a progress bar when cardano-db-v1 ancillary file is downloaded

Pre-submit checklist

Branch
- Tests are provided (if possible)
- Crates versions are updated (if relevant)
- CHANGELOG file is updated (if relevant)
- Commit sequence broadly makes sense
- Key commits have useful messages
PR
- All check jobs of the CI have succeeded
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested
Documentation
- Update documentation website (if relevant)
- No new TODOs introduced

Issue(s)

Relates to #2362

mithril-client/tests/extensions/snapshot_archives.rs

github-actions · 2025-03-19T13:45:21Z

Test Results

3 files ± 0 57 suites ±0 11m 38s ⏱️ +3s
1 778 tests + 6 1 778 ✅ + 6 0 💤 ±0 0 ❌ ±0
2 184 runs +14 2 184 ✅ +14 0 💤 ±0 0 ❌ ±0

Results for commit 80668fe. ± Comparison against base commit 81e90f7.

This pull request removes 14 and adds 20 tests. Note that renamed tests count towards both.

mithril-aggregator ‑ artifact_builder::cardano_immutable_files_full::tests::snapshot_archive_name_after_beacon_values
mithril-aggregator ‑ artifact_builder::cardano_immutable_files_full::tests::snapshot_archive_name_after_compression_algorithm
mithril-aggregator ‑ file_uploaders::local_snapshot_uploader::tests::should_copy_file_to_target_location
mithril-aggregator ‑ file_uploaders::local_snapshot_uploader::tests::should_error_if_path_is_a_directory
mithril-aggregator ‑ file_uploaders::local_snapshot_uploader::tests::should_extract_digest_to_deduce_location
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database::complete_without_start_should_not_panic
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database::completed_immutable_downloads_bump_progress
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database::start_including_ancillary_add_one_to_total_downloads
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database::start_then_complete_should_remove_multi_progress_bar
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database::starting_ancillary_downloads_should_add_a_progress_bar
…

mithril-aggregator ‑ artifact_builder::cardano_immutable_files_full::tests::snapshot_archive_name_include_beacon_and_network_values
mithril-aggregator ‑ file_uploaders::dumb_uploader::tests::get_history_of_multiple_uploads
mithril-aggregator ‑ services::snapshotter::compressed_archive_snapshotter::tests::snapshot_all_completed_immutables::include_only_completed_immutables
mithril-aggregator ‑ services::snapshotter::test_doubles::tests::dumb_snapshotter::set_dumb_snapshotter_archive_size
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database_v1::start_and_progress_ancillary_download_with_a_size_of_zero_should_not_crash
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database_v1::start_then_complete_should_remove_ancillary_progress_bar
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database_v1::start_then_complete_should_remove_immutables_progress_bar
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database_v1::starting_full_immutables_and_ancillary_together_spawn_two_progress_bars
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database_v2::complete_without_start_should_not_panic
mithril-client-cli ‑ utils::feedback_receiver::tests::cardano_database_v2::completed_immutable_downloads_bump_progress
…

♻️ This comment has been updated with latest results.

…related messages Co-authored-by: Damien Lachaume <[email protected]>

This will supersede `snapshot_all` since for v1 snapshots the ancillary files will be separated into another archive. Co-authored-by: Damien Lachaume <[email protected]>

Instead of including the ancillary files. Also suppress the now unused `snapshot_all` from the `Snapshotter` trait. Co-authored-by: Damien Lachaume <[email protected]>

… creation in cdb v1 artifact builder In order to re-use the name for the ancillary archive later. Co-authored-by: Damien Lachaume <[email protected]>

…of only the last Co-authored-by: Damien Lachaume <[email protected]>

… builder Co-authored-by: Damien Lachaume <[email protected]>

…oads Superseding the previously used `LocalSnapshotUploader`. This is needed because the previous uploaders return the same location for two differents path that contains a same digest, meaning that the ancillary location was targetting the immutable location in snapshot artifacts. Co-authored-by: Damien Lachaume <[email protected]>

By splitting the `fake` module into several files so they can be browsed more easily. Co-authored-by: Damien Lachaume <[email protected]>

…fake ancillary archive Co-authored-by: Damien Lachaume <[email protected]>

Co-authored-by: Damien Lachaume <[email protected]>

…db v1 snapshot + add new feedback events related to this download. Co-authored-by: Damien Lachaume <[email protected]>

…heck Co-authored-by: Damien Lachaume <[email protected]>

sfauvel

LGTM

sfauvel · 2025-03-19T16:12:12Z

mithril-client-cli/src/commands/cardano_db/download.rs

@@ -151,7 +151,7 @@ impl CardanoDbDownloadCommand {
        CardanoDbDownloadChecker::ensure_dir_exist(db_dir)?;
        if let Err(e) = CardanoDbDownloadChecker::check_prerequisites_for_archive(
            db_dir,
-            cardano_db.size,
+            cardano_db.size + cardano_db.ancillary_size.unwrap_or(0),


Is it make sense to create a function on Snapshot that compute this total size ?

sfauvel · 2025-03-19T16:51:55Z

mithril-common/src/entities/snapshot.rs

    pub size: u64,

-    /// Locations where the binary content of the snapshot can be retrieved
+    /// Size of the ancillary files in Bytes


I wonder if there is a structure that can be created with size + locations which can be used for immutable and ancillary (with an option).
It may be too much work to do that without a real value.

jpraynaud

LGTM 🚀

examples/client-cardano-database/src/main.rs

CHANGELOG.md

jpraynaud · 2025-03-20T14:18:51Z

mithril-client/src/snapshot_client.rs

@@ -126,7 +127,7 @@ pub struct SnapshotClient {
    #[cfg(feature = "fs")]
    http_file_downloader: Arc<dyn FileDownloader>,
    #[cfg(feature = "fs")]
-    feedback_sender: FeedbackSender,
+    _feedback_sender: FeedbackSender,


Could you explain why this field is prefixed with an underscore? Is it to avoid a breaking change in the API?

A comment was added to clarify the reason for the underscore prefix.

jpraynaud · 2025-03-20T14:21:33Z

mithril-client/tests/extensions/fake/cardano_db_v1.rs

+        // Ugly horror needed to update the snapshot location after the server is started, server
+        // which need said snapshot to start and run in another thread.
+        // The RwLock is needed to mutate the value across threads and the Arc to transfer it
+        // to the server thread while keeping an access on the main thread.


Maybe this comment is not really relevant:

Suggested change

// Ugly horror needed to update the snapshot location after the server is started, server

// which need said snapshot to start and run in another thread.

// The RwLock is needed to mutate the value across threads and the Arc to transfer it

// to the server thread while keeping an access on the main thread.

The comment was removed.

mithril-client/tests/extensions/fake/mod.rs

jpraynaud · 2025-03-20T15:12:16Z

mithril-aggregator/src/artifact_builder/cardano_immutable_files_full.rs

+        debug!(self.logger, ">> create_ancillary_snapshot_archive");
+
+        let snapshotter = self.snapshotter.clone();
+        let snapshot_name = format!("ancillary-{base_file_name_without_extension}");


I guess that this format is better as it will be easy to see files related to the same snapshot when listed alphabetically:

Suggested change

let snapshot_name = format!("ancillary-{base_file_name_without_extension}");

let snapshot_name = format!("{base_file_name_without_extension}-ancillary");

Done, we modified the name by adding .ancillary at the end to both meet your request and the function that extracts the digest from the file name:

let snapshot_name = format!("{base_file_name_without_extension}.ancillary");

…illary' at the end, and removal of the explanation comment about the technical reason for the use of RwLock in the test tooling of client integration tests.

… `SnapshotClient` API compatibility

Alenar assigned Alenar and dlachaume Mar 19, 2025

github-advanced-security bot found potential problems Mar 19, 2025

View reviewed changes

Alenar force-pushed the ensemble/2362/cdb_v1/separate_location_for_ancillary branch from 6883aba to a3294c6 Compare March 19, 2025 14:16

Alenar and others added 13 commits March 19, 2025 15:18

feat: add ancillary_locations and ancillary_size to all snapshot …

8b34a07

…related messages Co-authored-by: Damien Lachaume <[email protected]>

feat(aggregator): add snapshot_all_completed_immutables to Snapshotter

84b70e8

This will supersede `snapshot_all` since for v1 snapshots the ancillary files will be separated into another archive. Co-authored-by: Damien Lachaume <[email protected]>

feat(aggregator): snapshot only completed immutables in cardano db v1

523e3a2

Instead of including the ancillary files. Also suppress the now unused `snapshot_all` from the `Snapshotter` trait. Co-authored-by: Damien Lachaume <[email protected]>

refactor(aggregator): compute archive name separatly from the archive…

4cbb607

… creation in cdb v1 artifact builder In order to re-use the name for the ancillary archive later. Co-authored-by: Damien Lachaume <[email protected]>

test(aggregator): make DumbUploader store multiple uploads instead …

34c3f7a

…of only the last Co-authored-by: Damien Lachaume <[email protected]>

feat(aggregator): snapshot & upload ancillary in immutable files full…

4b94663

… builder Co-authored-by: Damien Lachaume <[email protected]>

test(client-lib): reorganize integration tests infrastructure

9468b49

By splitting the `fake` module into several files so they can be browsed more easily. Co-authored-by: Damien Lachaume <[email protected]>

test(client-lib): only add last ledger file and remove volatile from …

a2ab589

…fake ancillary archive Co-authored-by: Damien Lachaume <[email protected]>

test(client-lib): update cardano db v1 test to ancillary archive split

76f965c

Co-authored-by: Damien Lachaume <[email protected]>

fix(client-cli): incorrect genesis vkey in shared dev.json param file

9065e6c

Co-authored-by: Damien Lachaume <[email protected]>

feat(client-lib+cli): download ancillary archive if found in cardano …

4ea7f74

…db v1 snapshot + add new feedback events related to this download. Co-authored-by: Damien Lachaume <[email protected]>

feat(client-cli): include ancillary size in prerequisite disk space c…

e582efe

…heck Co-authored-by: Damien Lachaume <[email protected]>

Alenar force-pushed the ensemble/2362/cdb_v1/separate_location_for_ancillary branch from a3294c6 to e582efe Compare March 19, 2025 14:18

Alenar temporarily deployed to testing-preview March 19, 2025 14:27 — with GitHub Actions Inactive

Alenar requested review from jpraynaud and sfauvel March 19, 2025 15:04

Alenar marked this pull request as ready for review March 19, 2025 15:05

chore: update changelog

23bf434

Alenar temporarily deployed to testing-preview March 19, 2025 15:31 — with GitHub Actions Inactive

sfauvel approved these changes Mar 19, 2025

View reviewed changes

jpraynaud approved these changes Mar 20, 2025

View reviewed changes

dlachaume added 2 commits March 21, 2025 14:32

refactor: rephrasings, change ancillary archive naming by moving 'anc…

1af6d60

…illary' at the end, and removal of the explanation comment about the technical reason for the use of RwLock in the test tooling of client integration tests.

refactor(client-lib): rename fake module to fake_aggregator

f2da5a6

dlachaume force-pushed the ensemble/2362/cdb_v1/separate_location_for_ancillary branch from 5581b05 to f2da5a6 Compare March 21, 2025 13:33

dlachaume temporarily deployed to testing-preview March 21, 2025 13:42 — with GitHub Actions Inactive

chore(client-lib): add comment to clarify underscore prefix usage for…

80668fe

… `SnapshotClient` API compatibility

dlachaume temporarily deployed to testing-preview March 21, 2025 14:45 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Separate location for ancillary for cardano database v1 #2380

feat: Separate location for ancillary for cardano database v1 #2380

Alenar commented Mar 19, 2025 •

edited

Loading

github-actions bot commented Mar 19, 2025 •

edited

Loading

sfauvel left a comment

sfauvel Mar 19, 2025

sfauvel Mar 19, 2025

jpraynaud left a comment

jpraynaud Mar 20, 2025

dlachaume Mar 21, 2025

jpraynaud Mar 20, 2025

dlachaume Mar 21, 2025

jpraynaud Mar 20, 2025

dlachaume Mar 21, 2025 •

edited

Loading

	let snapshot_name = format!("ancillary-{base_file_name_without_extension}");
	let snapshot_name = format!("{base_file_name_without_extension}-ancillary");

feat: Separate location for ancillary for cardano database v1 #2380

Are you sure you want to change the base?

feat: Separate location for ancillary for cardano database v1 #2380

Conversation

Alenar commented Mar 19, 2025 • edited Loading

Content

Details

open-api & mithril-common

mithril-aggregator

mithril-client

mithril-client-cli

example-cardano-database (v1)

Pre-submit checklist

Issue(s)

github-actions bot commented Mar 19, 2025 • edited Loading

Test Results

sfauvel left a comment

Choose a reason for hiding this comment

sfauvel Mar 19, 2025

Choose a reason for hiding this comment

sfauvel Mar 19, 2025

Choose a reason for hiding this comment

jpraynaud left a comment

Choose a reason for hiding this comment

jpraynaud Mar 20, 2025

Choose a reason for hiding this comment

dlachaume Mar 21, 2025

Choose a reason for hiding this comment

jpraynaud Mar 20, 2025

Choose a reason for hiding this comment

dlachaume Mar 21, 2025

Choose a reason for hiding this comment

jpraynaud Mar 20, 2025

Choose a reason for hiding this comment

dlachaume Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Alenar commented Mar 19, 2025 •

edited

Loading

github-actions bot commented Mar 19, 2025 •

edited

Loading

dlachaume Mar 21, 2025 •

edited

Loading