Fix snapshot disk space check to run before download

[svlachakis]

Changes

• The snapshot plugin's free disk space check ran only at the extraction stage but demanded 2.5x the snapshot size. At that point the archive is already on disk (and is deleted right after extraction), so the multiplier double-counted the download: a volume effectively needed ~3.5x the snapshot to pass. A node with enough space for the download plus the extracted database would complete a multi-hour download and then crash-loop at extraction. For a ~2TB snapshot the check demanded ~5TB free after the ~2TB were already spent on the archive.
• The full download + extraction budget is now checked before the download starts: the snapshot size is probed with a ranged request that reads only the response headers (same Range and redirect handling as the downloader), and the requirement is the remaining download bytes plus a 1.5x extraction estimate. The check runs outside the download retry loop, so insufficient space aborts immediately instead of retrying forever. If the server does not report a size or the probe fails transiently (including a probe timeout), the pre-download check is skipped with a warning and the download proceeds as before.
• When Snapshot.SnapshotDirectory and Init.BaseDbPath live on different volumes, the pre-download check charges the remaining download bytes to the archive volume and the extraction estimate to the database volume; when they share a volume the requirements are summed, and when drive detection yields nothing the combined check against the database volume is kept as a conservative fallback.
• The extraction-stage check now demands only the 1.5x extraction estimate.
• New Nethermind.Init.Snapshot.Test project (wired into the CI test matrix) covering: extraction proceeding with free space between 1.5x and 2.5x (fails against the old guard), extraction rejected below the estimate, the download rejected upfront with no bytes fetched when the volume cannot fit download plus extraction, the requirement arithmetic for fresh and resumed downloads, and the two pre-check skip paths completing end to end (server reporting no Content-Length, and a transient probe failure followed by a successful download).

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

[claude]

Claude finished @svlachakis's task in 8m 49s —— View job

---

Review: snapshot disk space check before download

• Read PR diff and full context of changed files
• Analyze correctness / edge cases
• Check repo rules adherence
• Post review (6 inline comments)

The core diagnosis is right and the fix is well-targeted. The old 2.5x guard ran at extraction, when the archive was already on disk and about to be deleted, so it double-counted the download and demanded ~3.5x of the volume overall. Splitting it into (total − existing) before the download and 1.5x at extraction is the correct decomposition, and putting the check outside the retry loop so insufficient space aborts instead of retrying forever is the right call. Reusing SendWithRangeAsync for the probe — rather than a fresh request that would drift from the downloader's redirect and Range handling — is a good instinct. GetRequiredSpaceForDownload/ForExtraction are pure and directly tested, and the headline regression test genuinely fails against the old guard.

Below are the issues worth resolving. Nothing here is Critical; the two Mediums are about the new guard failing open or failing wrong rather than about the arithmetic.

---

High

1. The new test project will never run in CI. .github/workflows/nethermind-tests.yml enumerates test projects explicitly in its matrix (lines 54–108), and Nethermind.Init.Snapshot.Test isn't in it. Adding the project to Nethermind.slnx gets it compiled by build-test-artifacts.yml, but the test job runs dotnet test --no-build per matrix entry, so these tests are dead weight until the entry is added. I can't edit files under .github/workflows/ myself (GitHub App permissions), so this one needs a manual push — add - Nethermind.Init.Snapshot.Test to the project: list, keeping alphabetical order between Nethermind.History.Test and Nethermind.JsonRpc.Test.

Medium

2. A probe timeout escapes the graceful-skip filter and aborts node startup. (inline) The catch filter covers IOException or HttpRequestException, but HttpClient's default 100 s timeout surfaces as TaskCanceledException. Because SendWithRangeAsync uses ResponseHeadersRead, that timeout covers exactly the headers phase this probe consists of — so a snapshot host that is merely slow to respond fails the IStep with A task was canceled instead of skipping the check as documented.

3. One transient probe failure silently disables the guard entirely. (inline) The download has exponential backoff; the probe gets a single attempt. A 503 or connection reset → Warn → no check at all → the node downloads the full snapshot and can still die at extraction, which is the failure mode this PR set out to eliminate. Retrying the probe a few times, and logging the skip at Error, would keep the protection meaningful.

4. The requirement sums bytes destined for two volumes but checks only one. (inline) drives is keyed to Init.BaseDbPath, but the archive is written to Snapshot.SnapshotDirectory, which defaults to the relative path "snapshot" resolved against the process CWD. The common container layout (app on the image filesystem, BaseDbPath on a mounted data volume) puts them on different volumes, where the check over-demands 2.5x − existing on the DB volume and never examines the volume the archive actually lands on. IFileSystem.GetDriveInfos(string) already exists if you want to check each component against its own volume; otherwise the same-volume assumption should be stated in a <remarks>.

Low

5. GetTotalSizeAsync isn't headers-only, and has no XML doc. (inline) It's a GET whose body is abandoned on dispose — against a server that ignores Range it opens a full-file transfer and aborts it, and it kills the pooled connection that DownloadAsync would otherwise reuse. Suggested doc comment in the inline. The PR description's "headers-only request" should be corrected too.

6. The 1.5 multiplier deserves the one line of rationale that makes the fix legible. (inline) That 2.5 = 1.0 archive + 1.5 extracted is the entire insight here and appears nowhere in the code. Separately, worth confirming against a real published snapshot that a .zip mainnet archive expands by less than 1.5x — the old over-strict value would have masked it if not.

7. Test nits. (fixture, coverage) Silent catch (Exception) { return; } in the listener loop; racy random port selection; and no coverage for the two branches that let the download proceed unchecked (totalSize is null, probe throws) — the branches most likely to rot into a no-op guard.

Things I checked that are fine

• Dropping the if (drives.Length == 0) return; early exit is behaviour-preserving — the foreach over an empty array is a no-op.
• No overflow risk in (long)(snapshotSize * 1.5) or the sum; a 2 TB snapshot is far inside double's exact-integer range.
• The pre-download check sits outside the retry loop, so its IOException isn't caught by the catch (Exception e) when (e is IOException …) handler and retried forever. This is load-bearing and correct.
• totalSize for the resumed 206 (existing + contentLength) and non-range 200 (contentLength) cases both yield the full remote size, so GetRequiredSpaceForDownload is right in both. The 416 path returning existingSize gives 1.5x on an already-complete file, also correct.
• The probe is correctly gated behind the checkpoint.Read() >= SnapshotStage.Downloaded guard, so restarts past the download stage don't hit the network.
• The test csproj matches repo convention (tests.props import, no explicit TFM); Testably.Abstractions flows transitively from Nethermind.Core, and explicit using System; etc. are needed since the test project doesn't enable ImplicitUsings.

Note: dotnet build / dotnet test require approval in this environment, so I could not compile or execute the new tests — the review above is static analysis only. Please confirm the suite passes locally.

---

Verdict: not mergeable as-is — findings 2, 3, and 4 should be addressed or explicitly acknowledged with rationale, and finding 1 (CI matrix) means the tests this PR adds currently provide no protection.
· branch fix/snapshot-disk-space-stage

[svlachakis]

@claude review again

[svlachakis]

included in #12862