fullhistory: archive tip + captive-core backfill for no-lake deployments (#833) - #850
Conversation
8a10b76 to
604a021
Compare
A frontfill-only deployment (no [backfill.datastore]) had no tip source on
first start: the only source was notConfiguredTip, which always errored, so no
earliest_ledger form could pin a floor even though captive core could ingest
from it once pinned.
Consolidate all network-tip sampling into one tipSampler (Sample(ctx), retry
defaults bound at construction), replacing the (NetworkTipBackend, TipBackoff,
TipMaxAttempts) trio threaded through validateConfig, resolveEarliestFirstStart,
StartConfig, and backfillToTip. The sampler queries its sources in order and
returns the first that answers:
- the bulk lake frontier (backend.Tip) when a datastore is configured, and
- the history archives' root HAS (GetRootHAS().CurrentLedger) as the fallback,
and the sole source for a frontfill-only daemon.
The archive URLs are the same [ingestion].history_archive_urls captive core
already needs — no new config. Because the sampler now falls back to the
archives, an unavailable tip means no source was reachable at all, so the
synthetic tip = lastCommitted degraded mode in backfillToTip is deleted: on a
tip failure the pass errors and the supervisor restarts. The archives' one-
checkpoint lag is absorbed by the existing anchor = max(tip, lastCommitted) and
the signed withinOneChunkOfTip, so no lag handling is added.
f707825 to
914cabb
Compare
…ured A frontfill-only daemon with no [ingestion].history_archive_urls previously built an empty sampler and only died later at captive-core open, with a less on-point message; buildTipSampler now rejects the misconfiguration at startup. Also note at the sub-genesis gate that it is source-order-blind: a source must error (not return 0) when empty or it shadows a healthy fallback.
|
The archive tip is the right first half, but I think #850 should go one step further and let a no-lake deployment actually backfill, because otherwise the tip it can now resolve is only usable with With this PR a frontfill-only daemon can pin a below-now floor (the archive tip validates it in The type captiveSource struct {
ledgerbackend.LedgerStream // NewCaptiveCoreStream(coreConfig, log)
archives rootHASGetter
}
func (s *captiveSource) Tip(ctx context.Context) (uint32, error) { return archiveTip(s.archives)(ctx) }
The "Not a Backend" reasoning is right for a pure-frontfill deployment that never freezes bulk history, but it inverts once we want backfill: a captive-core-backfill deployment does freeze bulk history, so it needs the The one real cost is that each It also settles the tip wiring in one place: Net, the tip and the captive-core backfill are two halves of the same capability, and since the tip alone only unlocks |
The archive tip alone made a below-now earliest_ledger floor pinnable but not fillable: backfillSource step (3) errors on a backend-less chunk, so the only truly runnable no-lake config was "now". Close the gap with captiveSource — a backfill.Backend whose LedgerStream is the core opener's captive stream and whose Tip is the archives' root HAS. The SDK stream builds a fresh core per bounded RawLedgers call (ephemeral working dirs), so executePlan's parallel per-chunk replays run independent cores. Cost is one core catch-up per chunk: fine for a small retention window; deep history stays on the bulk lake. The primary tip is now Backend.Tip uniformly (lake for bsbSource, archive for captiveSource); the standalone archiveTip remains only as the lake's fallback and is never double-registered. On the no-lake path the core opener resolves before validateConfig (its stream IS the backend); the other paths keep the opener after validateConfig as before. The #833 acceptance test boots the real entrypoint with no datastore, a file:// archive (real pool construction + root-HAS frontier read), and a core stream serving the bounded replay, and asserts chunk 0 freezes exactly as on the lake path.
|
Done in 0912052 — you're right that the tip alone only unlocked What landed, matching the sketch:
One ordering consequence worth flagging: on the no-lake path the core opener now resolves before The new acceptance test boots the real entrypoint with no datastore, a |
| core := opts.Core | ||
| if backend == nil { | ||
| built, cleanup, berr := buildBackfillBackend(ctx, cfg, logger) | ||
| if core == nil && cfg.Backfill.DataStore.Type == "" && pool != nil { |
There was a problem hiding this comment.
Since every fullhistory daemon runs the live ingestion loop, core is needed regardless of the backfill source, so it can be resolved once up front instead of split between this early conditional and the late fallback at line 185:
core := opts.Core
if core == nil {
core, err = newCaptiveCoreOpener(cfg.Ingestion, cfg.Service.DefaultDataDir, logger)
if err != nil {
return err
}
}
backend := opts.Backend
if backend == nil {
built, cleanup, berr := buildBackfillBackend(ctx, cfg, core, pool, logger)
// ...
}That drops this compound predicate and the duplicated newCaptiveCoreOpener call, removes the late-resolution block, and lets buildBackfillBackend's guard shrink to if pool == nil since core is always present. The only behavior change is that a bad [ingestion].captive_core_config would surface before a bad earliest_ledger on a lake deployment, which is cosmetic since both are fatal startup errors. Non-blocking.
There was a problem hiding this comment.
Done in 49a7a81 — core now resolves once, right after the catalog opens, and the late fallback block is gone. buildBackfillBackend's guard shrank to if pool == nil with the non-nil-core contract noted in its doc.
The ordering change you called cosmetic did surface in one test: TestRunDaemon_NowFloorRequiresTip relied on validateConfig's "now" error firing before the stub captive_core_config was ever parsed; it now injects a fake Core so it keeps pinning the validateConfig failure it was written for.
tamirms
left a comment
There was a problem hiding this comment.
Now that captiveSource lands, the tip is effectively a property of the backend: bsbSource.Tip is the lake's export frontier, captiveSource.Tip is the archive's live frontier, and waitForCoverage already reads Backend.Tip directly rather than through the sampler. That leaves tipSampler doing two separable jobs: a retry-and-sub-genesis wrapper around a tip source (real, but that is a function), and a multi-source fallback list, which after the dedup is exercised only by the lake-primary-archive-fallback case.
That fallback no longer earns the abstraction, though to be clear it is not a correctness problem. If the anchor over-shoots what the lake holds, waitForCoverage just times out on the first uncovered chunk and the pass restarts and re-samples, the same graceful failure as any range that runs past the lake, and the retention floor follows lastCommitted so nothing over-prunes. It is marginal rather than wrong: if the lake's FindLatestLedgerSequence (a LIST) is failing, its ledger GETs are almost certainly failing too, so the archive anchor rarely buys real progress, and the retry already covers transient blips.
So the sampler should collapse into sampleWithRetry(ctx, backend.Tip): validateConfig and backfillToTip wrap backend.Tip with the retry, and coverage keeps reading it raw as it does today. That removes the tipSampler type, buildTipSampler's source-list assembly, StartConfig.Tip (callers reach the backend directly), and the backend.(*captiveSource) type-assertion, which only exists to avoid double-adding the archive the fallback introduces.
This composes with the inline note on the core resolution rather than replacing it: both shrink the same runDaemonWith block, but the core early/late split is independent, since captiveSource still needs the opener regardless of how the tip is sampled. It does reverse the lake-to-archive fallback from the #820 review threads, but captiveSource is exactly what changes that calculus: with every backend now owning its own frontier, the fallback stops paying for the sampler machinery. This should land in this PR rather than carry the extra type and the type-assertion forward.
…e once With captiveSource landed, the tip is a property of the backend: bsbSource exports the lake frontier, captiveSource the archives' root HAS, and the coverage wait already reads Backend.Tip raw. That left tipSampler doing two separable jobs — a retry-and-sub-genesis wrapper (kept, as the sampleTipWithRetry function; semantics unchanged) and a multi-source fallback list whose only remaining user was lake-primary-archive-fallback. Drop the fallback: a lake whose LIST fails has failing GETs too, an anchor overshoot just times out coverage and re-samples on restart, and the retention floor follows lastCommitted so nothing over-prunes. Gone with it: the tipSampler type, buildTipSampler, StartConfig.Tip (validateConfig takes the raw tipSource; backfillToTip reaches the backend directly), and the captiveSource type-assertion the dedup needed. The no-source fail-fast moves to runDaemonWith (nil backend errors at startup with the same config-shaped message). The core opener also resolves once up front — every daemon runs the live ingestion loop — replacing the early/late split; buildBackfillBackend's guard shrinks to the pool check. A bad [ingestion] config now surfaces before a bad earliest_ledger, which is cosmetic: both are fatal. Test fakes that drove failing tips through validateConfig/backfillToTip switch to sub-genesis tips (permanent not-ready fails in one poll), so they don't sleep through the production backoff those call sites apply.
|
Done in 49a7a81 — Removed with it, as listed: the One consequence worth noting: with the fallback gone, "no tip source configured" can no longer be detected at sampler build, so the equivalent fail-fast moved to Test fallout: the three multi-source |
|
Two tests slipped past the sub-genesis switch and now sleep ~4s each. The collapse hardcodes the production backoff (
Each adds ~4s to the suite, and they re-exercise retry exhaustion that |
|
|
||
| // Tip reports the archives' current frontier — the root HAS CurrentLedger. | ||
| func (s *captiveSource) Tip(ctx context.Context) (uint32, error) { | ||
| return archiveTip(s.archives)(ctx) |
There was a problem hiding this comment.
Small leftover from removing the fallback: archiveTip now has a single production caller, this method, and archiveTip(s.archives)(ctx) builds a tipSource closure only to invoke it immediately. That closure shape existed so the multi-source sampler could hold it in a source list; with the sampler gone it can fold into the method:
func (s *captiveSource) Tip(ctx context.Context) (uint32, error) {
has, err := s.archives.GetRootHAS()
if err != nil {
return 0, fmt.Errorf("history archive root HAS: %w", err)
}
return has.CurrentLedger, nil
}That drops the archiveTip function and the build-a-closure-then-call-it indirection, and TestArchiveTip_RootHASErrorSurfaces becomes a test of captiveSource.Tip with the same coverage. tipSource the type stays as the param abstraction for sampleTipWithRetry and validateConfig; only its "returned by archiveTip" use goes away.
There was a problem hiding this comment.
Done in 425208e — archiveTip is folded into captiveSource.Tip exactly as sketched, and the build-a-closure-then-call-it indirection is gone. The 64-ledger checkpoint-lag note (anchor max(tip, lastCommitted) + signed withinOneChunkOfTip absorb it) moved onto the method so it stays with the code it describes. rootHASGetter moved next to captiveSource too, since Tip is now its only consumer — tip.go is left holding just tipSource and sampleTipWithRetry. TestArchiveTip_RootHASErrorSurfaces became TestCaptiveSourceTip_RootHASErrorSurfaces with the same coverage.
|
@codex[agent] review |
… tests
archiveTip's closure shape existed so the multi-source sampler could hold
it in a source list; with the sampler gone its one production caller was
captiveSource.Tip building the closure only to invoke it. Inline it (the
checkpoint-lag note moves onto the method) and move rootHASGetter next to
its now-sole consumer — tip.go is left holding just tipSource and
sampleTipWithRetry.
TestBackfill_RestartTipUnavailableErrors (né ...Unreachable...) and
TestRun_FirstStartNoTipErrors slipped past the sub-genesis switch and
slept through the production backoff (~4s each) on erroring tips; both
now use tips{0} — one permanent "not ready" poll, same assertions,
~0.1s. Retry exhaustion stays covered by
TestSampleTip_ExhaustedRetriesErrors at a 1ms interval.
|
Done in 425208e — both switched to sub-genesis fakes ( Took the softening option on the name: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49a7a81930
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pool, err := historyarchive.NewArchivePool(urls, historyarchive.ArchiveOptions{ | ||
| ConnectOptions: storage.ConnectOptions{Context: ctx}, | ||
| Logger: logger.WithField("subservice", "history-archive"), | ||
| }) |
There was a problem hiding this comment.
Verify archive tip against the configured network
In the no-lake path this archive pool is the Tip source used by validateConfig, but it is created without the captive-core NETWORK_PASSPHRASE. If history_archive_urls accidentally points at another Stellar network and earliest_ledger = "now", GetRootHAS can still return that archive's height, PinEarliestLedger persists a floor derived from the wrong network, and only the later captive-core replay fails the passphrase check; after fixing the URL the immutable catalog pin remains wrong and requires wiping. Please pass the configured network passphrase into this pool or otherwise verify the archive before using its root HAS to pin config.
Useful? React with 👍 / 👎.
Closes #833.
A no-lake deployment (
[backfill.datastore].typeempty) had no tip source on first start: the only source wasnotConfiguredTip, which always errors, so noearliest_ledgerform could pin a floor. And a tip alone would only have made the floor pinnable, not fillable —backfillSourcestep (3) errors on a backend-less chunk, so the sole truly runnable no-lake config would have beenearliest_ledger = "now". Per the review discussion, both halves land together: the archive tip and captive-core backfill.The tip is a property of the backend. Every
backfill.Backendowns its frontier — the lake's export frontier forbsbSource(datastore.FindLatestLedgerSequence), the archives' root HAS forcaptiveSource— and all three consumers read it:validateConfigandbackfillToTipthroughsampleTipWithRetry(ctx, backend.Tip, …), the freeze'swaitForCoverageraw.sampleTipWithRetryis the single home for the retry semantics, moved verbatim from the oldnetworkTiphelper: cenkalti/backoff constant interval, count-bounded, sub-genesis tip isbackoff.Permanent("not ready"), ctx cancellation aborts the wait. This replaces both the round-1(NetworkTipBackend, TipBackoff, TipMaxAttempts)trio and the interimtipSamplertype — the multi-source lake→archive fallback from the #820 threads is deliberately dropped (review rationale): once each backend owns its frontier, the fallback's only user was the lake, where a failing LIST almost always means failing GETs too, and an anchor overshoot just times out coverage and re-samples on restart. With it goStartConfig.Tipand thecaptiveSourcetype-assertion the dedup needed.captiveSource: no-lake backfill through captive core. With no datastore but[ingestion].history_archive_urlsconfigured (which live ingestion requires anyway — no new config),buildBackfillBackendreturns acaptiveSource: the core opener'sLedgerStreamplus the archives' root HAS asTip. Everything downstream runs unchanged —executePlan's parallel pool,backfillSource,WriteColdChunk, and thewaitForCoveragepoll onBackend.Tip. Parallel per-chunk replays compose because the SDK'scaptiveCoreStream.RawLedgersis a stateless factory: each bounded call builds a fresh core in its own ephemeral working dir, so N concurrentRawLedgers(BoundedRange(chunk))calls run N independent cores. The real cost is one core catch-up per chunk, which makes the retention-window size the natural selector: no-lake + archives suits a small window; deep history stays on the bulk lake.One core opener, resolved up front. Every daemon runs the live ingestion loop, so the opener resolves once before the backend (whose no-lake path is built from its stream), replacing the early/late split. The one behavior change: a bad
[ingestion]config now surfaces before a badearliest_ledger— cosmetic, both are fatal startup errors.Archives absorb their own lag. The archives' one-checkpoint (64-ledger) lag is absorbed by the existing
anchor = max(tip, lastCommitted)and the signedwithinOneChunkOfTip; no lag handling is added.Synthetic tip deleted. An unavailable tip now means the backend's frontier query kept failing — so the
tip = lastCommitteddegraded mode inbackfillToTipis gone. A tip failure errors the pass and the supervisor restarts (a transient outage self-heals on re-sample); the daemon never serves behind an unknown frontier.Tests: the #833 acceptance test boots the real entrypoint with no datastore, a
file://archive (real pool construction + root-HAS frontier read), and a core stream serving the bounded replay, and asserts chunk 0's cold artifacts freeze exactly as on the lake path;captiveSourceconstruction + frontier mapping; a fail-fast startup error when neither source is configured; the retry/sub-genesis/ctx-cancel suite onsampleTipWithRetry; andTestBackfill_RestartTipUnavailableErrorsreplacing the deleted degraded-mode test. Fullfullhistory-shortsuite and-raceon thefullhistorypackage pass; repo golangci-lint clean on the diff.Deliberate skip: the archive pool is built without a network passphrase, so the SDK's HAS passphrase check is off. A wrong-network archive URL already fails loudly at live ingestion — captive core consumes the same
[ingestion].history_archive_urls— with recovery being a wipe of the still-empty first-start data dir. (With the opener now resolved before the pool this is wireable if wanted; left out to keep the change to the reviewed scope.)