backfill: cap prefetch memory per chunk task - #942
Conversation
eb50588 to
9ef8387
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
3b7a33f to
ba44559
Compare
There was a problem hiding this comment.
Pull request overview
Bounds BSB backfill prefetch using worker-adjusted byte and download budgets while retaining a fixed object-depth limit.
Changes:
- Adds shared budget calculation and coverage.
- Exposes byte limits through daemon and benchmark configuration.
- Pins the SDK implementation providing
BufferBytes.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Pins the required SDK revision. |
go.sum |
Updates dependency checksums. |
cmd/stellar-rpc/rpcv2/sample_config_test.go |
Updates sample defaults. |
cmd/stellar-rpc/rpcv2/rpc-v2-sample-config.toml |
Documents BSB budget controls. |
cmd/stellar-rpc/internal/rpcv2/config/flags.go |
Adds int64 flag support. |
cmd/stellar-rpc/internal/rpcv2/config/config.go |
Resolves and exports byte budgets. |
cmd/stellar-rpc/internal/rpcv2/config/config_test.go |
Updates configuration expectations. |
cmd/stellar-rpc/internal/rpcv2/config_validate.go |
Rejects negative byte budgets. |
cmd/stellar-rpc/internal/rpcv2/bench/sources.go |
Applies benchmark pool shares. |
cmd/stellar-rpc/internal/rpcv2/bench/hot.go |
Uses single-stream budgeting. |
cmd/stellar-rpc/internal/rpcv2/bench/command.go |
Adds the byte-budget flag. |
cmd/stellar-rpc/internal/rpcv2/bench/cold.go |
Uses cold worker-pool sizing. |
cmd/stellar-rpc/internal/rpcv2/backfill/backend.go |
Implements shared budget policy. |
cmd/stellar-rpc/internal/rpcv2/backfill/backend_test.go |
Tests budget calculation and limits. |
Suppressed comments (1)
cmd/stellar-rpc/rpcv2/rpc-v2-sample-config.toml:257
BufferBytesis not a hard memory cap: the pinned SDK derives queue depth from the most recently consumed object's buffer capacity, while in-flight downloads, pooled buffers, and size changes can exceed the target. Calling it prefetch memory can cause operators to treat 256 MiB as a resident-memory ceiling; describe it as an approximate queued-payload target instead.
# buffer_bytes: prefetch MEMORY for one stream, and the bound that tracks
# memory — a pubnet object grew from 295 bytes in early history to ~210 KB near
# the tip, so an object count says nothing about bytes. Unset takes an equal
# share of a 256 MiB pool-wide budget.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
422498e to
f309141
Compare
f309141 to
99fe087
Compare
99fe087 to
fc6ba31
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (1)
cmd/stellar-rpc/internal/rpcv2/config_validate.go:212
- This comment no longer matches the pinned SDK:
NewBufferedStorageBackendnow rejects a negativeBufferBytesvalue rather than treating it as an unbounded budget. Keep the daemon-side check for its config-specific message and no-side-effects validation, but document that rationale so future maintainers do not rely on obsolete SDK behavior.
// A negative budget reaches the SDK verbatim and disables the bound. The
// Unset (nil pointer, filled with the pool share by WithDefaults) and zero
// (cap off) are the two valid ways to not set a byte budget; a negative is
// always a mistake, so fail it at config load with a message naming both.
c6487c1 to
01472ca
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Suppressed comments (2)
cmd/stellar-rpc/internal/rpcv2/config_validate.go:213
- This rationale and remediation are inaccurate. The pinned SDK rejects negative
BufferBytesrather than treating it as disabled, and this validator also handles CLI overrides, where deleting a TOML key does not remove--backfill.bsb.buffer_bytes=-1. Keep the early validation, but describe the SDK behavior correctly and tell users to omit the key or flag.
// A negative budget reaches the SDK verbatim and disables the bound. The
// Unset (nil pointer, filled with the default by WithDefaults) and zero
// (cap off) are the two valid ways to not set a byte budget; a negative is
// always a mistake, so fail it at config load with a message naming both.
if bytes := deref(bsb.BufferBytes); bytes < 0 {
cmd/stellar-rpc/rpcv2/rpc-v2-sample-config.toml:257
- The public configuration reference is now stale:
design-docs/full-history-streaming-workflow.md:120-127omitsbuffer_bytes, still documentsnum_workersas 50, and describesbuffer_sizein ledgers rather than datastore objects. Update that table with this public configuration change so operators do not size backfill from the old semantics.
#buffer_bytes = 33_554_432 # 32 MiB
01472ca to
35495fb
Compare
35495fb to
1f01f69
Compare
a405abb to
8dac820
Compare
BSB prefetch was 5,000 objects per task with 50 downloads each, and a pubnet object grew ~700x across history — so the setting costing ~12 MB at genesis cost ~8.4 GB near the tip on an 8-core box, doubled by GOGC: issue #895's memory finding. Cap each task's prefetch in BYTES instead (the SDK's new BufferBytes, stellar/go-stellar-sdk#5988), defaulting to 32 MiB, and drop the default download concurrency to 25 — 12-50 measure identically, and aggregates to 1,600 concurrent requests are error-free. The object count stays as a per-object-overhead bound. Every [backfill.bsb] key is per chunk task; totals multiply by [backfill].workers. Measured on 16 pubnet tip chunks at 8 workers, GOMEMLIMIT unset: 26.49 GB -> 7.1/7.3 GB peak RSS at identical throughput. Closes the prefetch half of #895; the other memory driver there (index builds at NumCPU/2) is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zyTXU8wkocJgN6mBafnou
8dac820 to
7798386
Compare
Note
stellar/go-stellar-sdk#5988 (
BufferBytes) is merged;go.modpins its merge commit (f479ee93).What
BSB prefetch was 5,000 objects per chunk task with 50 concurrent downloads each, and a pubnet object grew ~700× across history — so the setting costing ~12 MB at genesis cost ~8.4 GB near the tip on an 8-core box. Doubled by GOGC, that is #895's memory finding.
This caps each task's prefetch in bytes instead:
[backfill.bsb]keybuffer_bytes(new in the SDK)num_workersbuffer_sizeEvery key is per chunk task; totals multiply by
[backfill].workers. One rule on every surface: unset takes the default; there is no off switch —0and negatives are rejected at config load, since an uncapped prefetch is the configuration that caused #895. Set a huge value for effectively-unbounded.Results
16 pubnet tip chunks, 8 workers,
GOMEMLIMITunset, both sides built from this base ref:For scale, #895 measured this base at ~29 GB + 8 GB swap over full history. On the campaign branch (#902), whose memory work stacks with this, the same fix measures 5.8 GB.
Supporting measurements: throughput is flat across a 100× object-count range and an 8× byte-budget range; 12–50 downloads per task measure identically; and at 32 workers, aggregate download concurrency of 256 / 800 / 1,600 concurrent GCS requests is throughput-identical and error-free — which is why a flat per-task default is safe and no cross-worker budget pooling is needed.
Scope
The prefetch half of #895. The other memory driver named there — cross-window index builds at
NumCPU/2— is untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_013zyTXU8wkocJgN6mBafnou