feat: add --resume to continue an interrupted export - #11
Open
gacevicljubisa wants to merge 16 commits into
Open
feat: add --resume to continue an interrupted export#11gacevicljubisa wants to merge 16 commits into
gacevicljubisa wants to merge 16 commits into
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TestReadCursor never exercised a valid log line split across the 64 KiB backward-read window boundary in lastCursorPlain; every prior case had the target line either wholly inside the last window or was garbage that fails to parse regardless of reconstruction order. Add a case that deterministically positions a real line across the boundary and asserts the straddle before running, so a broken carry concatenation order regresses this case specifically.
Adds AppendWriter and AppendLogsAsync alongside the existing SaveLogsAsync so a resumed export can append to an existing NDJSON file instead of overwriting it, dropping logs a skip function (e.g. resume.Cursor.Skip) reports as already written. filestore takes a bare func(types.Log) bool rather than importing pkg/resume, keeping the dependency one-directional. Test helper fix beyond the brief: feed() must give each log a non-nil Topics slice — go-ethereum's generated Log.UnmarshalJSON rejects a null "topics" field as missing, so a nil Topics (the brief's literal feed()) fails blocksIn's round trip regardless of the implementation under test.
…pstore
Closes a gap in the plan: no task specified the append-then-re-read
round trip promised for pkg/resume, even though it needs the
filestore.AppendLogsAsync/AppendWriter and gzipstore.AppendWriter this
task completes. Adds TestAppendResumeRoundTrip, table-driven over
plain NDJSON and gzip, that builds a starting export with a genuinely
partial final block, resumes it through resume.Read + the matching
append writer + AppendLogsAsync(skip: cursor.Skip), and asserts:
- the cursor lands on the last saved log
- a boundary-block log at a higher index than the cursor is written,
not skipped, since it was never saved
- the full sequence after append is exactly original + new logs,
with no duplicates and nothing dropped
- the original bytes remain an unchanged prefix of the appended
file (for gzip, proof a genuine second member was added rather
than the archive being decompressed and rewritten)
- resume.Read on the appended file advances to the newly written
tail, so a resumed export is itself resumable
An interrupted run can leave a tail that no reader can trust: half an NDJSON line, a line that never got its newline, or a gzip member that never got its CRC and length trailer. Read tolerated all three and told its caller nothing, so the write path appended straight onto them and silently destroyed logs. A valid but unterminated last line was the worst case: the cursor pointed at it, so a resumed query skipped it, while the append fused it with the next log into one unparseable line and the entry was gone for good. The cursor now names the last entry that is complete and properly terminated, and reports the offset at which the file's recoverable content ends (CleanSize) together with whether anything follows it (Truncated). Safety does not rest on recovery: where no boundary can be positively identified, Read refuses with ErrNoCleanBoundary instead of guessing one. For gzip, walk the file a member at a time with Multistream(false) plus Reset over an io.ByteReader, so the reader is never wrapped in a bufio of its own and the byte count stays in step with its real position. That gives exact member boundaries with no new dependency. A member that ends mid-line is not treated as a boundary at all, since appending after it would glue the next log onto an unterminated one. lastCursorGzip also stops discarding scanner errors. Returning the last cursor is still right, but a truncated member, a checksum mismatch or an over-long line now leave the walk at the last clean boundary rather than yielding a too-early cursor with a nil error. That was the single line that made the corruption silent and endlessly repeatable: every later resume appended more unreachable data while reporting success. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AppendLogsAsync dropped the error from Close, though its doc comment promised a buffered destination is always flushed. On the gzip path Close writes the deflate terminator and the member footer, so a failure there -- a full disk is entirely plausible for a 90 MB export -- left a truncated member behind while the CLI printed "all logs have been saved". Join the close error into the result instead; errors.Join keeps errors.Is working, so a cancelled context still reads as context.Canceled. SaveLogsAsync now opens through CreateWriter and shares the same path, which fixes the same dropped Close for a fresh export and lets a caller open the destination before it starts producing logs. memberWriter.Close names the file it failed on, as CompressFile does; that error is about to become visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Discard whatever follows the resume file's clean boundary before the writer is opened, logging the offset and how many bytes went. Nothing recoverable is lost: everything discarded sits at or after the cursor, so the resumed query fetches it again. Only the offset the reader positively identified is ever truncated to, and never past it. This runs after the RPC connection is up, so an unreachable endpoint leaves the file untouched. Open the writer synchronously before GetLogs as well. Opening it inside the saving goroutine meant a failure there logged and returned while fetchLogs kept pushing onto the 100-slot channel; once full it blocked until cancellation, errorChan never closed, and RunE looped for ever printing "still retrieving logs..." while nothing was saved. It now returns the error immediately, and saveLogs is left as a dispatch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The claim that "entries already in the file are skipped, so resuming never duplicates or drops a log" held only for a file that ends cleanly. Say so, and document what happens when a run killed mid-write leaves a partial entry behind: the tool truncates to the last complete entry and re-fetches from there, or refuses outright when no such point can be identified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clarify that the resume boundary is the last newline-terminated line that parses as a log entry, not just any newline-terminated line. And scope the guarantee about re-fetching: for export content (partial writes), discarded data is re-fetched; for foreign data that was never a log entry, it is simply removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan was scaffolding for building the feature, not reference material for using it. The design rationale it carried now lives in the README and in the PR description. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comments had grown into design essays restating what the code already says. Keep only the reasoning that stops the code being broken by a well-meaning simplification -- why countingReader must implement io.ByteReader, why Multistream is switched off per member, why a line without a newline means an interrupted write, why the close error is joined -- and drop the narration around it. Also removes the step-by-step narration in CompressFile and stale references to the review that produced the tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
export --resume <file>so an interrupted export continues from where it stopped instead of restarting from the contract's start block.Design
.ndjson,.gz, and.gzipall work, which matters since this repo's own archives use.gzip.Cursor.Skip. No gaps, no duplicates.O_APPEND; gzip files get a new gzip member. Concatenated members are a valid gzip stream (RFC 1952), sogzcat,gunzip, and Go'scompress/gzipread the result as one continuous file and the existing ~90 MB is never rewritten. Verified against a copy of the real archive withgzcat,gunzip -t, Go, and Python.--resumeoverrides--startand--output, warning when either was set explicitly.--compressbecomes a no-op when resuming an already-compressed file.Recovering from an unclean shutdown
A hard kill (SIGKILL, OOM, short write) can leave a partial line or a gzip member with no trailer. Appending onto any of those corrupts the file, so the reader identifies the last offset at which the file is known complete — the end of the last newline-terminated line that parses as a log entry, or of the last whole gzip member — and the export truncates to it before appending, logging what it dropped:
This is lossless for export content: everything discarded sits at or after the resume point, so the resumed query re-fetches it. If no clean boundary can be identified, resume refuses (
ErrNoCleanBoundary) rather than guessing — the safety property does not depend on recovery succeeding.Structure
pkg/resumepkg/gzipstoreAppendWriter— appends a gzip member.CompressFileunchanged.pkg/filestoreAppendLogsAsyncwith a skip filter; sharedwriteLogscore.SaveLogsAsyncsignature unchanged.cmd/export.goDependency direction is one-way:
cmddepends on all three, none of the three depend on each other —filestoretakes a barefunc(types.Log) boolthatCursor.Skipsatisfies structurally.Also fixes a pre-existing bug: the
ctx.Done()path logged "waiting for logs to be saved..." and then returned without waiting, which could runCompressFileover a still-writing file.Testing
The repo previously had no tests; this adds the first, covering the tail reader's edge cases (truncated tails, multi-member gzip, a valid line straddling a 64 KiB window boundary), an append-resume round trip across all three packages, and a regression test for each of the three corruption scenarios above.
go test ./...,go vet ./..., andgolangci-lint runare clean.cmd/export.gohas no unit test —RunEneeds a live RPC endpoint and there's no HTTP fixture harness here — so the testable logic lives inpkg/. It was verified manually against copies of the real archives with the endpoint reachable.Known limitations
batch-archiveconsumes.--resumetarget that exists but holds no parseable entries fails rather than starting fresh.--compresswith a plain resume can overwrite an existing.gzipof the same name.🤖 Generated with Claude Code