Skip to content

fix(storage): do not enforce DirectPath for Rapid buckets - #4987

Open
alleaditya wants to merge 2 commits into
GoogleCloudPlatform:masterfrom
alleaditya:plan_suggestion_541764145
Open

fix(storage): do not enforce DirectPath for Rapid buckets#4987
alleaditya wants to merge 2 commits into
GoogleCloudPlatform:masterfrom
alleaditya:plan_suggestion_541764145

Conversation

@alleaditya

@alleaditya alleaditya commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

In GCSFuse, createGRPCClientHandle in internal/storage/storage_handle.go previously attached experimental.WithDirectConnectivityEnforced() unconditionally for all gRPC clients.

For Rapid buckets, DirectPath is not supported/enforced. Forcing DirectPath headers causes GCS servers to reject non-DirectPath requests once DirectPath enforcement is active.

This change:

  1. Conditionally omits experimental.WithDirectConnectivityEnforced() when isBucketRapid is true.
  2. Skips DirectPath connectivity verification (verifyDirectPathConnectivity) for Rapid buckets to avoid unnecessary dummy Stat API call retries, latency, and warning logs.
  3. Adds dedicated unit test cases for Rapid vs Non-Rapid bucket gRPC client handle initialization in internal/storage/storage_handle_test.go.

Link to the issue in case of a bug fix.

Fixes b/541764145

Testing details

  1. Manual - Ran verification script scratch/verify_grpc_headers.sh confirming header and DirectPath handling for Rapid vs Non-Rapid buckets.
  2. Unit tests - Added TestCreateGRPCClientHandle_RapidBucket and TestCreateGRPCClientHandle_NonRapidBucket in ./internal/storage. Tests pass cleanly (0.00s execution for Rapid bucket).
  3. Integration tests - Executed storage package test suite (go test -v ./internal/storage).
Branch File Size Read BW Write BW RandRead BW RandWrite BW
Master 0.25MiB 546.8MiB/s 1.26MiB/s 72.64MiB/s 1.27MiB/s
PR 0.25MiB 543.38MiB/s 1.29MiB/s 71.39MiB/s 1.26MiB/s
Master 48.828MiB 4454.94MiB/s 72.86MiB/s 1463.0MiB/s 62.66MiB/s
PR 48.828MiB 4437.52MiB/s 75.43MiB/s 1498.43MiB/s 75.68MiB/s
Master 976.562MiB 4463.69MiB/s 33.9MiB/s 698.35MiB/s 39.37MiB/s
PR 976.562MiB 4493.43MiB/s 37.92MiB/s 1085.05MiB/s 39.9MiB/s

Any backward incompatible change? If so, please explain.

No

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates createGRPCClientHandle to skip DirectPath enforcement and verification for rapid buckets, while also correcting the casing of the isBucketRapid parameter. Additionally, unit tests are added to verify this behavior. The feedback suggests using require.NoError and require.NotNil in the new tests to fail immediately and simplify the client cleanup logic.

Comment thread internal/storage/storage_handle_test.go Outdated
@alleaditya alleaditya added execute-perf-test Execute performance test in PR execute-integration-tests Run only integration tests labels Aug 4, 2026
@alleaditya
alleaditya force-pushed the plan_suggestion_541764145 branch 2 times, most recently from dedd676 to 0eebca1 Compare August 4, 2026 08:00
@alleaditya
alleaditya marked this pull request as ready for review August 4, 2026 08:44
@alleaditya
alleaditya requested a review from a team as a code owner August 4, 2026 08:44
@meet2mky

meet2mky commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

How do you enforce the direct path for rapid when customer sets direct-path-only

Options: 'direct-path-only' (fail if unavailable), 'direct-path-with-fallback' (always fallback to HTTP/1 when direct path is not available).

@vadlakondaswetha This is not going to be handled correctly if this is not enforced. We have to fix the fallback here.

Comment thread internal/storage/storage_handle.go Outdated
@alleaditya alleaditya removed the execute-perf-test Execute performance test in PR label Aug 4, 2026
@alleaditya
alleaditya force-pushed the plan_suggestion_541764145 branch from 0eebca1 to 31784ba Compare August 4, 2026 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

execute-integration-tests Run only integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants