fix(wintermute): bound the repo backfill queue - #254
Open
afbase wants to merge 2 commits into
Open
Conversation
The relay enumerator is a producer with no consumer whenever backfiller workers are disabled, which is the recommended setting where repo backfill is handled elsewhere. Nothing bounded it, so the queue grew for as long as the relay had repos to list. Measured on a profiling box consuming the live firehose with workers disabled: the queue reached 5.8 million entries in under five hours and its Fjall partition 906 MB, against 7 MB for every other partition combined. The LSM read and compaction paths allocate block buffers in proportion, and those dominated the heap profile. Resident memory tracked the queue on every cycle and only a restart reclaimed it, which reads as a leak but is a queue nothing drains. Enumeration now stops at REPO_BACKFILL_MAX_QUEUE entries, default 250k, with 0 meaning unbounded for anyone who wants the previous behaviour. The bound is passed in rather than read from the environment inside the loop, so it can be tested without a relay, and the queue length is tracked rather than re-read: Fjall's len() scans the partition, so polling it per page would cost more the larger the queue got.
…ig constants
The call site used a fully-qualified path while the file already had a
use crate::config::{...} block for every other constant it reads.
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.
The relay enumerator is a producer with no consumer whenever backfiller workers are disabled, which is the recommended setting where repo backfill is handled elsewhere. Nothing bounded it, so the queue grew for as long as the relay had repos to list.
Measured on a box consuming the live firehose with workers disabled: the queue reached 5.8 million entries in under five hours, and its Fjall partition 906 MB against 7 MB for every other partition combined. A heap profile over the same period showed the LSM read and compaction paths —
LevelReader,segment::Range,MvccStream— holding roughly 3.5 GB of 4.8 GB peak live, allocating block buffers in proportion to the partition they were reading.Resident memory tracked the queue across every restart cycle:
That reads as a leak — memory climbs, only a restart reclaims it — but it is a queue nothing drains. Peak live memory itself was stable across two consecutive six-hour windows (4.82 GB then 4.44 GB) while half a terabyte of allocation churned through, so nothing was being retained that should not have been.
Enumeration now stops at
REPO_BACKFILL_MAX_QUEUEentries, default 250k.0restores the previous unbounded behaviour.Two details worth review:
len()scans the partition, so polling it per page would grow more expensive as the queue did. A concurrent drain only makes the tracked value an overestimate, which errs toward enqueueing less.This does not change behaviour for anyone running backfiller workers unless their queue legitimately exceeds the bound, in which case enumeration pauses until it drains.
Test plan
cargo clippy -p rsky-wintermute --all-targets --no-deps -- -D warningscargo test -p rsky-wintermuteagainst a database with the appview schemacargo build --release -p rsky-wintermuteBACKFILLER_WORKERS=0,ingester_repo_backfill_lengthstops at the bound instead of climbingREPO_BACKFILL_MAX_QUEUE=0reproduces the previous unbounded behaviour