Skip to content

refactor(mithril-client): Optimize cardano db artifacts download #2359

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

Merged
merged 9 commits into from
Mar 11, 2025

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Mar 7, 2025

Content

This PR refactor the way cardano database v2 files are downloaded in the mithril-client:

  • Unified download system for immutables in ancillary:
    • allowing them to be in the same join set
    • download tasks are now precomputed
  • Fail early: as soon as the last location fails when downloading a files (instead of failing at the very end of the process)
  • Continuously download max_parallel_downloads files: No more threshold effect because of chunking

Performance improvement over a download of ~10 000 immutables without ancillary

Note

Those commands are run on a 1 Gbit/s download speed internet connection

Before, 10k immutables download took ~33 seconds:

$./mithril-client --unstable --run-mode dev cardano-db-v2 download latest --start 7345
1/6 - Checking local disk info…
2/6 - Fetching the certificate and verifying the certificate chain…
  Certificate chain validated
3/6 - Downloading and unpacking the cardano db snapshot
   [00:00:33] [############################################################################################] Files: 10,006/10,006 (0.0s)
4/6 - Computing and verifying the Merkle proof…
5/6 - Computing the cardano db snapshot message
6/6 - Verifying the cardano db signature…
Cardano database snapshot '5845e9e1232f91c804e483eda1e7af6b9816174d0410923da6f032c1c1c91f5f' archives have been successfully unpacked.

After, 10k immutables download took ~19 seconds:

$./mithril-client --unstable --run-mode dev cardano-db-v2 download latest --start 7345
1/6 - Checking local disk info…
2/6 - Fetching the certificate and verifying the certificate chain… 
  Certificate chain validated
3/6 - Downloading and unpacking the cardano db snapshot
   [00:00:19] [############################################################################################] Files: 10,006/10,006 (0.0s)
4/6 - Computing and verifying the Merkle proof…
5/6 - Computing the cardano db snapshot message
6/6 - Verifying the cardano db signature…
Cardano database snapshot '5845e9e1232f91c804e483eda1e7af6b9816174d0410923da6f032c1c1c91f5f' archives have been successfully unpacked.

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
    • No new TODOs introduced

Comments

This PR also fix the clippy warnings that were raised when checking with only the fs feature enabled.

Issue(s)

Closes #2327

@Alenar Alenar added the refactoring 🛠️ Code refactoring and enhancements label Mar 7, 2025
@Alenar Alenar self-assigned this Mar 7, 2025
Copy link

github-actions bot commented Mar 7, 2025

Test Results

    3 files  ± 0     56 suites  ±0   10m 26s ⏱️ +8s
1 726 tests +13  1 726 ✅ +13  0 💤 ±0  0 ❌ ±0 
2 122 runs  +17  2 122 ✅ +17  0 💤 ±0  0 ❌ ±0 

Results for commit 037d9c5. ± Comparison against base commit 39e5a58.

This pull request removes 5 and adds 18 tests. Note that renamed tests count towards both.
mithril-client ‑ cardano_database_client::download_unpack::tests::download_unpack_ancillary_file::download_unpack_ancillary_files_fails_if_location_is_unknown
mithril-client ‑ cardano_database_client::download_unpack::tests::download_unpack_immutable_files::download_unpack_immutable_files_fails_if_location_is_unknown
mithril-client ‑ cardano_database_client::proving::tests::download_unpack_digest_file::fails_if_location_is_unknown
mithril-client ‑ file_downloader::interface::tests::immutable_files_location_to_file_downloader_uris
mithril-client ‑ file_downloader::interface::tests::immutable_files_location_to_file_downloader_uris_return_error_when_location_is_unknown
mithril-client ‑ cardano_database_client::download_unpack::tests::download_unpack_ancillary_file::building_ancillary_download_tasks_fails_if_all_locations_are_unknown
mithril-client ‑ cardano_database_client::download_unpack::tests::download_unpack_immutable_files::building_immutables_download_tasks_fails_if_all_locations_are_unknown
mithril-client ‑ cardano_database_client::proving::tests::download_unpack_digest_file::fails_if_all_locations_are_unknown
mithril-client ‑ utils::vec_deque_extensions::tests::pop_all_elements_leave_source_vec_empty
mithril-client ‑ utils::vec_deque_extensions::tests::pop_more_than_source_vec_size_is_equivalent_to_popping_all_elements
mithril-client ‑ utils::vec_deque_extensions::tests::pop_one_element_return_vec_with_the_first_element
mithril-client ‑ utils::vec_deque_extensions::tests::pop_zero_element_return_empty_vec
mithril-common ‑ entities::file_uri::tests::expand_multi_file_template_to_immutable_file_number
mithril-common ‑ entities::file_uri::tests::expand_multi_file_template_to_one_file_uri
mithril-common ‑ messages::cardano_database::tests::sanitize_ancillary_locations::fails_if_all_locations_are_unknown
…

♻️ This comment has been updated with latest results.

@Alenar Alenar temporarily deployed to testing-preview March 7, 2025 16:58 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-preview March 7, 2025 17:28 — with GitHub Actions Inactive
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator

@dlachaume dlachaume left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Alenar added 6 commits March 10, 2025 17:49
- It will allow to put the ancillary task in the join set
- Trials of the locations are now done at the task level instead of
  being at the joinset level. This means that the process will fails
  as soon as one download have all its locations fails instead of
  failing at the end of all downloads.
as it's easier to work with our current case:
- it allow to pop front so downloads task are done using the task order
  withoug having to reverse a vector (because we can only `pop` a vec
  and not `pop_front`).
- it will be more easy to add the ancillary task at the start of the
  list.
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM

@Alenar Alenar force-pushed the djo/2327/optimize_cardano_db_artifacts_download branch from 05fee6e to a9f79ae Compare March 10, 2025 16:49
Alenar added 3 commits March 10, 2025 17:55
Those methodes filters out all `Unknown` locations and fails if they
were all `Unknown`
* mithril-client from `0.11.13` to `0.11.14`
* mithril-common from `0.5.10` to `0.5.11`
@Alenar Alenar force-pushed the djo/2327/optimize_cardano_db_artifacts_download branch from a9f79ae to 037d9c5 Compare March 10, 2025 16:57
@Alenar Alenar temporarily deployed to testing-preview March 10, 2025 17:06 — with GitHub Actions Inactive
@Alenar Alenar merged commit 62ae79e into main Mar 11, 2025
37 of 38 checks passed
@Alenar Alenar deleted the djo/2327/optimize_cardano_db_artifacts_download branch March 11, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring 🛠️ Code refactoring and enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize artifact downloads when restoring an Incremental Cardano DB
4 participants