Skip to content

Conversation

@meet2mky
Copy link
Collaborator

Description

This pull request introduces a mechanism to reuse GCS read handles within the BufferedReader. By reusing the read handle, we can optimize the creation of subsequent GCS readers for the same GCS object, which is particularly beneficial for zonal buckets as it can bypass authorization and metadata checks.

Changes

  • A new ReadHandle struct is introduced to store the read handle and its expiration time (5 mins)
  • BufferedReader now uses an atomic.Pointer to the ReadHandle struct for thread-safe access and updates
  • The read handle is updated by the downloadTask after a successful read from a zonal bucket after which it will be refreshed if it's expired.
  • Unit tests have been added to verify the behaviour of the read handle logic, including initial state, updates on expiration, and no updates when the handle is still valid.

Performance

Sequential Reads on RAPID

Master: upto 4.2 GB/sec
PR: upto 4.5 GB/sec

New Reader Creation

Master (AVG: 22ms):
image

PR (AVG: 15ms):
image

Link to the issue in case of a bug fix.

Testing details

  1. Manual - Yes
  2. Unit tests - Yes
  3. Integration tests - Part of pre-submit.

Any backward incompatible change? If so, please explain.

@meet2mky meet2mky self-assigned this Nov 28, 2025
@meet2mky meet2mky added execute-integration-tests Run only integration tests execute-integration-tests-on-zb To run E2E tests on zonal bucket. labels Nov 28, 2025
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.26%. Comparing base (5e24904) to head (a14299c).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4086       +/-   ##
===========================================
+ Coverage        0   83.26%   +83.26%     
===========================================
  Files           0      153      +153     
  Lines           0    18612    +18612     
===========================================
+ Hits            0    15498    +15498     
- Misses          0     2544     +2544     
- Partials        0      570      +570     
Flag Coverage Δ
unittests 83.26% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

defaultPrefetchMultiplier = 2
ReadOp = "readOp"
readOp = "readOp"
readHandleValidity = 5 * time.Minute // source go/gcsfuse-zonal-buckets-read-design-doc
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please confirm, if this expiry time is client specific? Or server maintains a shared handle and refresh every 5 mins?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Expiry time is based on per buffered Reader instance. I couldn't find any resources apart from it's mention in the source go/gcsfuse-zonal-buckets-read-design-doc

@meet2mky meet2mky force-pushed the meet2mky/reuse-read-handle-in-buffered-reader branch from 8d3531c to a14299c Compare December 1, 2025 06:09
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 execute-integration-tests-on-zb To run E2E tests on zonal bucket.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants