feat(ingest): rewrite backfill as a streaming fetch → process → flush pipeline#631
Open
aditya1702 wants to merge 6 commits into
Open
feat(ingest): rewrite backfill as a streaming fetch → process → flush pipeline#631aditya1702 wants to merge 6 commits into
aditya1702 wants to merge 6 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
- gofumpt re-aligns ingestService/Configs structs after the field removals/rename - drop test fixtures orphaned by the deleted batch tests (keypair vars, ledgerMetadataWith1Tx) - annotate the errgroup g.Wait() return (//nolint:wrapcheck) since stage errors are already wrapped
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.
TL;DR
Rewrites historical-ledger backfill (
internal/services/ingest_backfill.go) from a per-batch worker pool into a single streaming fetch → process → flush pipeline built on theoptimizedStorageBackendfork introduced in #622. Net −1709 lines.fetchLedgersgoroutine → boundedrawCh→ N process workers → boundedresultCh→ one ordered flusher. Coordinated byerrgroup.optimizedStorageBackend(10 decode workers + prefetch buffer) every ~250 ledgers — there's now exactly one per gap.ProcessLedgerTransactionswhen the pool is nil) instead of N batch workers each fanning out onto a shared unboundedpondpool.--backfill-workers; decode parallelism stays in the fork'snum-workers.fetchLedgersis the soleGetLedgercaller, ascending, one backend per gap.completed[]/watermarkIdx/MarkDone) and its lower bound are gone. The ordered flusher emits a monotonic "safe-end" close time; the compressor compressesrange_end < safeEnd, coalescing bursts viadrainLatest.startBackfillingreturns the error (previously it logged per-batch failures and returnednil).synchronous_commit=off); a rerun resumes viaGetLedgerGaps.--backfill-batch-size; rename--backfill-db-insert-batch-size→--backfill-flush-size.--backfill-workers. Pre-release, so no compat shim.Two correctness properties are load-bearing and were explicitly verified in review: (1) TimescaleDB
range_endis exclusive, so compressingrange_end < safeEndnever compresses a chunk before all its ledgers are durable; (2) the compressor's safe-end stays monotonic across gaps becausecalculateBackfillGapsreturns gaps in ascending order.Why
Backfill replays historical ledgers into TimescaleDB. The previous design layered three uncoordinated concurrency mechanisms — a
pondbatch pool, a freshBufferedStorageBackendper 250-ledger batch, and an unbounded shared indexer pool — with fetch/decode/process/flush running serially inside each batch. That churns backends, oversubscribes goroutines, and leaves cores idle during every fetch stall and DB flush. This rewrite collapses it to one idiomatic pipeline with a single parallelism axis, reusing the decode pipeline theoptimizedStorageBackendfork already provides.Suggested review order
internal/services/backfill_pipeline.go— the pipeline:orderedBuffer,fetchLedgers,processWorker,runFlusher,runGapPipeline(theerrgroupwiring + channel-close discipline).internal/services/ingest_backfill.go—startBackfilling(validate → gaps → per-gap pipeline →compressor.Wait()on all paths);calculateBackfillGapsis unchanged.internal/services/backfill_compressor.go— the stateless progressive compressor +drainLatest.internal/indexer/indexer.go— the nil-pool serial path (live pooled path is byte-for-byte unchanged).internal/services/ingest.go,internal/ingest/ingest.go,cmd/ingest.go— theprocessLedgersplit + config rename.Design context:
~/.claudeproject spec/plan (out of repo).Reviewer notes / before the in-cluster benchmark
--backfill-batch-size/--backfill-db-insert-batch-sizeand use--backfill-flush-size, or it errors on an unknown flag.--backfill-workers=8— theNumCPUdefault may exceed the ~8-process GC sweet spot on a high-core node; co-tune with the fork'snum-workers(decode) per the earlier ~16/8 split.--compression-max-chunksflag implies a TimescaleDB columnstore policy/job may already compress chunks in-cluster. If so, the app-level progressive compressor may partly overlap (safe —compress_chunkis idempotent on disjoint chunks — but possibly redundant). Worth confirming whether the app compressor is still wanted.Concurrency notes
ctx.Done(), so first-error cancellation can't deadlock on a fullrawChor an unreadresultCh.rawChis closed by its sole sender (the fetcher);resultChis closed by a single fan-in goroutine afterwg.Wait()— no double-close, no send-on-closed on the error path.compressor.Wait()on both the success and gap-error paths.Test plan
go build ./...+go vet ./...— clean.ENABLE_INTEGRATION_TESTS=false go test -raceoninternal/services,internal/indexer,internal/ingest— pass (fullservicessuite included, exercising the shared-processLedgerlive path).orderedBufferreorder (out-of-order → contiguous runs),drainLatestcoalescing, indexer serial==pooled equivalence,startBackfillingfail-fast validation.make check(lint / deadcode / shadow) — via CI.🤖 Generated with Claude Code