Skip to content

feat(solana): CAR download script#1982

Merged
sistemd merged 6 commits intomainfrom
sistemd/car-file-persistence
Mar 16, 2026
Merged

feat(solana): CAR download script#1982
sistemd merged 6 commits intomainfrom
sistemd/car-file-persistence

Conversation

@sistemd
Copy link
Contributor

@sistemd sistemd commented Mar 16, 2026

No description provided.

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Review of the Solana CAR download script and solana_compare tracing improvements.

Key findings (by priority):

  1. Performance — blocking sync I/O in async context: fs_err::exists, fs_err::create_dir_all, and fs_err::metadata are blocking calls on async worker threads. Use tokio::fs equivalents.

  2. Panic branch: .expect("invalid range header") should return an error instead of panicking.

  3. Durability for resume: flush() doesn't guarantee data persistence to disk. Use sync_all() since the resume logic depends on accurate on-disk file sizes.

  4. No integrity verification: Only file size is checked for completeness. A corrupted or truncated download would not be detected.

  5. No reqwest timeout: Default client has no connect/read timeout, meaning a stalled connection could hang indefinitely without triggering a retry.

  6. Guideline violations: Missing doc comments on CarDownloadError, abbreviated log field names, non-standard duration logging format, and #[from] usage on error variants.

  7. DRY: env_filter_or_info is duplicated verbatim across two example files.

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Follow-up: additional logging guideline findings from docs/code/logging-errors.md.

@sistemd sistemd force-pushed the sistemd/car-file-persistence branch from b20c5f0 to 415df97 Compare March 16, 2026 18:26
@sistemd sistemd requested a review from leoyvens March 16, 2026 18:27
Copy link
Collaborator

@leoyvens leoyvens left a comment

Choose a reason for hiding this comment

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

Object storage support will be important

@sistemd sistemd changed the title feat(solana): car download script feat(solana): CAR download script Mar 16, 2026
@sistemd sistemd merged commit c44d59a into main Mar 16, 2026
8 checks passed
@sistemd sistemd deleted the sistemd/car-file-persistence branch March 16, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants