Repository navigation
persist: read GCS blobs in concurrent 8 MiB ranges - #40486
Merged
alex-hunt-materialize merged 3 commits intoNov 10, 2026
Merged
alex-hunt-materialize merged 3 commits into
alex-hunt-materialize merged 3 commits into
Conversation
alex-hunt-materialize
force-pushed
the
persist-gcs-chunked-get
branch
4 times, most recently
from
November 5, 2026 11:04
3d5536f to
8f81af3
Compare
GcsBlob::get streamed each blob over a single HTTP/1.1 connection. It now reads the first 8 MiB to learn the size and generation, then fetches the remaining ranges concurrently, pinned to that generation. In us-east1 a 128 MiB get went from ~500ms to ~125ms, faster than S3Blob over GCS's S3 API with the HTTP pool (~600ms). The SDK skips checksum validation for range reads, so get now verifies the object's CRC32C itself, using the new direct crc32c dependency. Adds gcs_blob_get_chunk_boundaries. Co-Authored-By: Claude Opus 5.5 <[email protected]>
alex-hunt-materialize
force-pushed
the
persist-gcs-chunked-get
branch
from
November 5, 2026 11:07
8f81af3 to
2fa5c77
Compare
Times 128 MiB gets and sets through a single-stream read, GcsBlob, and, when HMAC credentials are set, S3Blob against GCS's S3 API. Co-Authored-By: Claude Opus 5.5 <[email protected]>
alex-hunt-materialize
marked this pull request as ready for review
November 5, 2026 12:08
mtabebe
approved these changes
Nov 5, 2026
mtabebe
left a comment
mtabebe
left a comment
Contributor
There was a problem hiding this comment.
One small comment, but otherwise LGTM
| } | ||
| // The SDK skips checksum validation for range reads, so check the | ||
| // whole object's CRC32C from the first response's metadata. | ||
| if let Some(expected_crc32c) = crc32c { |
Contributor
There was a problem hiding this comment.
Is there a valid case where we don't have an object's CRC in the first response metadata?
Or does the lack of a checksum indicate that the metadata we got back was garbage? If so, should we also error in the none case?
Contributor
Author
There was a problem hiding this comment.
Good catch! We were silently skipping it 🤦. GCS only includes the CRC when the range covers the whole object. I just pushed a fix.
GCS sends x-goog-hash only when the requested range covers the whole object, so multi-chunk gets silently skipped the CRC32C check. Fetch the CRC32C from the object's metadata, pinned to the same generation and concurrently with the remaining chunks, and error if there is none. 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.
GcsBlob::getstreamed each blob over one HTTP/1.1 connection (the SDK forces HTTP/1.1, so the HTTP/2 pool from #39906 doesn't apply). It now reads the first 8 MiB to learn the size and generation, then fetches the remaining ranges concurrently, pinned to that generation. The SDK skips checksum validation for ranged reads, sogetchecks the total length and verifies the object's CRC32C itself, hashing each chunk as it arrives. This adds a directcrc32cdependency, already inCargo.locktransitively.128 MiB, n2-standard-16 in us-east1-b, median of 10:
GcsBlob, single stream (before)GcsBlob, chunked (this PR)S3Blobover GCS S3 API, pool onWrites remain slower than
S3Bloband are left for a follow-up.Tests:
gcs_blob_get_chunk_boundariescovers empty objects (GCS rejects ranges on them with 416, whichgetmaps to an empty result) and sizes around chunk boundaries.gcs_blob_throughputis an ignored bench producing the numbers above.The two commits are easier to review individually: the first changes
get, the second adds the bench.🤖 Generated with Claude Code