fix(storage): qualify presence store lookups by bucket so the primary key serves them - #579
Merged
Merged
Conversation
… key serves them presenceForCIDs filtered blob_presence on key alone, but the primary key is (bucket, key). On Postgres 15 that predicate cannot use the index, so every repair batch read the whole table. On a store-all node that is ~4.8M rows per 1000-upload batch, and the cost is a CPU-bound filter of each row against the key array rather than I/O, so it does not improve as the table warms. Measured on a 4.8M-row copy: ~0.7-1s per batch when the planner inlines the array and hashes it, 100s+ when it runs as a bare parameter under a generic plan. Naming both bucket labels in the predicate keeps the "fetch both buckets, let Lookup pick" semantics and turns the query into a handful of index probes (~100ms on the same copy). Adds a test that a row recorded under the archive label resolves through the same query, since a label missing from that list would read as absent. Co-Authored-By: Claude Fable 5.1 <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
1 participant
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.
Summary
presenceForCIDsfilteredblob_presenceonkeyalone, but the primary key is(bucket, key). On Postgres 15 that predicate cannot use the index, so every repair batch seq-scanned the whole table — ~4.8M rows per 1000-upload batch on a store-all node. The cost is a CPU-bound filter of each row against the key array, not I/O, so it does not improve as the table warms.This adds
bucket = any($1)naming both bucket labels. The "fetch both buckets and letLookuppick" semantics are unchanged; the query becomes a handful of primary-key index probes.Measurements
On a 4.8M-row copy of the table in the mediorum test Postgres (15), one batch of 1,360 keys:
key = any($1), array inlined as a Constkey = any($1), bare Param (generic plan)bucket = any($1) and key = any($2)blob_presence_pkeyThe first row is what pgx's custom plans give today, so on
audius.rickyrombo.comthis is a ~1s tax per batch rather than the main cost. The second row is the cliff this removes: if the planner ever switches that statement to a generic plan, each batch would spend minutes in the query.Test plan
make test-mediorumpassesTestPresenceForCIDsCoversEveryBucketLabel: a row recorded under the archive label resolves through the same query, so a label left out of the predicate's list cannot silently read as "missing"🤖 Generated with Claude Code