Skip to content

feat: cleanup unexpected files in immutable folder after download #2502

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 8 commits into from
May 22, 2025

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented May 21, 2025

Content

This PR add a two phase tool in the mithril-client that:

  1. compute the expected state of the immutable folder after download
  2. remove any file or directory that is not in the expectation
  3. report offenders in a warning log

The removal is executed even if the download fail.

caveats

  • The tool can be tricked in some specific case, i.e: download of a Range of immutable, if a downloaded immutable archive contains a immutable below the downloaded range it won't be identified
  • The tool enforce the current directory structure of the Cardano node immutable directory: if the node change it's immutable naming scheme this tool will need to be adapted, else it will remove the files with the new scheme
  • The reported offenders are only in a warning log, if library users don't enable the logs they won't see the message

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

Issue(s)

Closes #2429

@Alenar Alenar self-assigned this May 21, 2025
Copy link

@Copilot 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

This PR introduces a new two-phase tool in mithril-client to compute the expected state of the immutable folder and remove any unexpected files after snapshot download, while logging file offenders.

  • Introduces the new module UnexpectedDownloadedFileVerifier in the utils package.
  • Modifies snapshot_client and internal_downloader to incorporate file removal after download (even in error cases).
  • Adjusts module exports and download routines to include ancillary verification and unexpected file cleanup.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
mithril-client/src/utils/mod.rs New module unexpected_downloaded_file_verifier added and re-exported to support file cleanup.
mithril-client/src/snapshot_client.rs Updated download and unpack routines to compute expected file state, remove unexpected files, and conditionally download ancillary files.
mithril-client/src/cardano_database_client/download_unpack/internal_downloader.rs Adjusted the download task flow to compute the expected state and remove unexpected files even if downloads fail.
Comments suppressed due to low confidence (1)

mithril-client/src/snapshot_client.rs:241

  • [nitpick] Consider adding a comment explaining that the removal step is deliberately executed even if the download fails, to clarify intended behavior for future maintainers.
expected_files_after_download.remove_unexpected_files().await?;

Copy link

github-actions bot commented May 21, 2025

Test Results

    3 files  ± 0     77 suites  ±0   15m 39s ⏱️ +41s
1 931 tests +13  1 931 ✅ +13  0 💤 ±0  0 ❌ ±0 
3 278 runs  +39  3 278 ✅ +39  0 💤 ±0  0 ❌ ±0 

Results for commit 1f4b4d7. ± Comparison against base commit c8f5ca0.

♻️ This comment has been updated with latest results.

@Alenar Alenar temporarily deployed to testing-preview May 21, 2025 14:16 — with GitHub Actions Inactive
@Alenar Alenar force-pushed the djo/2429/cleanup-unexpected-immutables-after-download branch from 324a0ba to 364e497 Compare May 21, 2025 16:03
@Alenar Alenar temporarily deployed to testing-preview May 21, 2025 16:13 — with GitHub Actions Inactive
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 👍

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 👍

Alenar added 8 commits May 22, 2025 12:10
A two stages tools that first compute the expected state of a cardano
database immutable folder after download, then after said download is
done remove all unexpected files.
…root

And take in account the fact that directories (root or immutable) could
not exist before download.
…work on non existing dir

As the download step may have not been able to create them because of a
failure.
* mithril-client from `0.12.5` to `0.12.6`
@Alenar Alenar force-pushed the djo/2429/cleanup-unexpected-immutables-after-download branch from 364e497 to 1f4b4d7 Compare May 22, 2025 10:11
@Alenar Alenar temporarily deployed to testing-preview May 22, 2025 10:19 — with GitHub Actions Inactive
@Alenar Alenar merged commit 3e925f5 into main May 22, 2025
38 checks passed
@Alenar Alenar deleted the djo/2429/cleanup-unexpected-immutables-after-download branch May 22, 2025 15:01
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.

Cleanup unexpected immutable files in archive in client
4 participants