Fail a read that delivers fewer bytes than the store holds

[benelser]

What and why

RedisStore::get_part reads a value in read_chunk_size GETRANGE calls
and treats any chunk shorter than the chunk size as the end of the data.
GETRANGE on a missing key returns an empty string, so a key that is
evicted or replaced between two chunks ends the stream with a clean EOF
after the bytes already sent. GcsStore::get_part forwards the HTTP
body and sends EOF when the body ends, with no check against the object
size. RedisStore::update ignores UploadSizeInfo::ExactSize, so a
populate stream that ends early is renamed in as a shorter value under
the real key and every later read serves it as complete. Under a verify -> lz4 -> fast_slow{redis, gcs} chain a VerifyStore cannot catch any
of this on the read side, and the caller sees Got EOF earlier than expected from the compression reader with no store error in the chain.
Measured on a v1.6.3 deployment: 11 such truncated ByteStream.Read
calls in 24 h on blobs from 454 KB to 50.8 MB, each one a build-fatal
materialize_inputs_failed for Buck2, which does not retry them.

The stores now know how many bytes a read owes before they stream it.
RedisStore::get_part reads STRLEN and EXISTS first (the same pipeline
has_with_results uses), clips the requested window to the stored
length, and requires every GETRANGE chunk inside that window to come
back whole; a short chunk returns Internal naming the key, the
expected and the delivered length, or NotFound when nothing has been
sent yet so a fast_slow caller can fall through to its slow store.
GcsStore::get_part resolves the object size once per call inside the
retry loop, counts the bytes delivered across retries, and treats a body
that ends early as a retryable error that resumes from the last offset;
a genuinely short object surfaces as that error once the retries are
spent. RedisStore::update rejects a stream whose length differs from
ExactSize before the STRLEN check and the RENAME, with
InvalidArgument naming both lengths. The digest size cannot serve as
the expected length here: under a compression store the stored value is
the compressed frame, so the length has to come from the store itself.

How was this verified?

New tests, one per invariant, in the existing test files.

In redis_store_test:

• get_part_errors_when_key_vanishes_mid_read: STRLEN says three
  chunks, the second GETRANGE returns "" -> Internal naming 3072
  expected and 1024 delivered.
• get_part_errors_when_chunk_is_shorter_than_window: a non-empty short
  chunk -> Internal naming 2048 expected and 1536 delivered.
• get_part_reports_not_found_when_key_vanishes_before_first_chunk:
  first GETRANGE returns "" -> NotFound.
• update_rejects_stream_shorter_than_exact_size: two bytes against
  ExactSize(4) -> InvalidArgument, no STRLEN, no RENAME.

In gcs_store_test:

• get_part_errors_on_short_body: the mock caps every body at 5 of 11
  bytes, no retry budget -> Internal naming both.
• get_part_resumes_after_short_body: with retries the read completes
  in three bodies (5 + 5 + 1) and one metadata call.

Without the store changes the five failure tests pass the truncated data
through as a clean EOF and fail on their assertions; the resume test
fails because the store sends EOF after 5 bytes. The existing tests were
updated for the up-front STRLEN/EXISTS call and for the size lookup that
now precedes a GCS content read; all of them pass.

Deployed on a development ring with the Redis fast tier capped at 256 MB
so it evicted continuously, with a fault injector that deleted the Redis
key or shrank it to a 4 KiB prefix partway through a chunked read. On
the unpatched image the injected mid-read eviction produced the clean-
EOF signature in 1163 of 1163 landed injections over three runs. On the
patched image the same fault produced the explicit short-read error 31
of 31 times and a clean EOF never. Under natural eviction alone on the
patched image (256 MB tier, 2.4 GB write flood, 2595 evictions) 1404
reads completed with 0 short. On a production deployment of the patched
build, 12 hours of live traffic (about 4 million CAS requests) produced
0 clean-EOF truncations and 1 explicit short-read error, on a 555 KB
blob whose Redis key was lost between chunks; the affected build failed
with the named digest instead of a truncated file, and GetActionResult
p99 stayed at 235 ms with no request at the 30 s deadline.

Risk

Default on; there is no knob because a read that returns fewer bytes
than the store holds is never correct. Behavior changes for every
deployment: a Redis read costs one extra STRLEN/EXISTS round trip and a
GCS read one metadata request per get_part call, both before the first
byte; a fast_slow populate already made the same two calls through
has, so on that path the count doubles from one to two each. Readers
of a key that shrinks or disappears mid-read now get an error where they
got a truncated success; readers of a genuinely short GCS object get an
error after the retry budget instead of the short body. A Redis populate
whose stream ends before ExactSize now fails instead of storing the
prefix. Callers that request a window past the end of a value still get
the clipped window, as before.

AI assistance

@benelser Please fill in this. It should contain which AI tools helped. "None" is a complete answer if that's the case.

---

This change is 

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
nativelink	Ready	Preview	Sep 30, 2026 10:46am UTC
nativelink-aidm	Ready	Preview	Sep 30, 2026 10:46am UTC

[benelser]

The asan failure after the branch update is grpc_store_test::update_splits_buffers_larger_than_the_grpc_message_limit hitting its own 5 s rpc_timeout_s under AddressSanitizer (GrpcStore::write RPC timed out after 5s, attempt 1, then DeadlineExceeded). This change touches only the Redis and GCS stores and their tests; no gRPC store code is in the diff. The same test passes locally on the merged head three times in about 0.13 s. I cannot re-run the job from a fork; a re-run should clear it, or that test's deadline may want a sanitizer-aware margin.

[benelser]

Could a maintainer re-run the asan job? The failure is the gRPC store test timeout described above, outside this diff, and I can't re-run it from a fork. Everything else is green and it is ready for review.

[palfrey]

Could a maintainer re-run the asan job? The failure is the gRPC store test timeout described above, outside this diff, and I can't re-run it from a fork. Everything else is green and it is ready for review.

This is probably the issue from #2818 so trying with an update

[palfrey]

@benelser I've fixed the ASAN bit, please fill in the AI assistance part of the description as that's new since this got opened originally.