Repository navigation
Conversation
…ssionStore::has() a1e6efb (TraceMachina#2960) made CompressionStore::has() report a digest key's declared size instead of the compressed stream's length, and left string keys reporting the stored length as a follow-up. This is that follow-up. A FastSlowStore declares the slow tier's has() size to its fast tier, and since 086289a (TraceMachina#2974) the populate itself rejects a fill whose length differs from that size. A string key read through a compression slow tier therefore still fails every populate on main: Expected 514 upload bytes, received 88064 Slow store ended before the cache fill was complete A string key has no size of its own, so the size comes from the footer. Its last three fields (uncompressed_data_sz u64, block_size u32, version u8) have fixed widths and always end the stream, so one ranged get_part of 13 bytes at the inner store's reported length finds it without reading the blocks; the version and block size are checked. If the object went away since the inner has(), the key reports missing. If the footer is not at the reported length, the inner store reported something other than the stream's exact length (FilesystemStore reports its block-rounded footprint), and locating the footer would mean reading the whole stream, so has() logs a warning and reports the stored length as before; reporting None instead would call a present object missing. String keys rarely reach a compression store, since CAS traffic is digest keyed, so the extra read stays off the FindMissingBlobs path. Tests: - has_reports_uncompressed_size_of_string_key: the footer path, plus an empty object (0) and a missing key. - populate_from_compression_slow_store_into_filesystem_fast_store: end to end for a digest key and a string key. On main the string key fails as above. - has_reports_uncompressed_size_of_digest: the digest path TraceMachina#2960 fixed, over a two-block payload and has_many with a zero and a missing digest. - has_preserves_missing_digests_and_stored_string_key_sizes (from TraceMachina#2960) pinned the stored length for a string key; it now expects the blob's length and is renamed to match. - has_mixed_digests_preserves_result_order and has_nonzero_digests_delegates_single_batch used string keys against a recording inner store that reports sizes it cannot serve. They now use digest keys. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
What and why
nativelink-store: report a string key's uncompressed size from CompressionStore::has()
a1e6efb (TraceMachina#2960) made CompressionStore::has() report a digest key's
declared size instead of the compressed stream's length, and left string
keys reporting the stored length as a follow-up. This is that follow-up.
A FastSlowStore declares the slow tier's has() size to its fast tier, and
since 086289a (TraceMachina#2974) the populate itself rejects a fill whose length
differs from that size. A string key read through a compression slow tier
therefore still fails every populate on main:
Expected 514 upload bytes, received 88064
Slow store ended before the cache fill was complete
A string key has no size of its own, so the size comes from the footer.
Its last three fields (uncompressed_data_sz u64, block_size u32, version
u8) have fixed widths and always end the stream, so one ranged get_part of
13 bytes at the inner store's reported length finds it without reading the
blocks; the version and block size are checked. If the object went away
since the inner has(), the key reports missing. If the footer is not at the
reported length, the inner store reported something other than the
stream's exact length (FilesystemStore reports its block-rounded
footprint), and locating the footer would mean reading the whole stream,
so has() logs a warning and reports the stored length as before; reporting
None instead would call a present object missing. String keys rarely reach
a compression store, since CAS traffic is digest keyed, so the extra read
stays off the FindMissingBlobs path.
The FastSlow half of the original branch (
beb2d5f872) is #75; this PR is the compression-store half only.How was this verified?
Tests:
empty object (0) and a missing key.
end for a digest key and a string key. On main the string key fails as
above.
a two-block payload and has_many with a zero and a missing digest.
pinned the stored length for a string key; it now expects the blob's
length and is renamed to match.
has_nonzero_digests_delegates_single_batch used string keys against a
recording inner store that reports sizes it cannot serve. They now use
digest keys.
Also part of the soak branch (#73): upstream HEAD plus every pending fix, run through the railgun fleet for an hour of continuous remote builds with no errors.
Risk
Only string keys through a compression store change; digest keys keep TraceMachina#2960's behaviour. One extra ranged read of 13 bytes per string-key
has(); CAS traffic is digest keyed, so FindMissingBlobs is unaffected. If the footer is not where the inner store's length says, the stored length is reported with a warning, as before. The companion FastSlow change is #75.AI assistance
Written with Claude Code; b7r6 reviewed the change. Draft until the author has read the CI results.
🤖 Generated with Claude Code