Skip to content

Conversation

@shubham-up-47
Copy link
Contributor

@shubham-up-47 shubham-up-47 commented Sep 18, 2025

This change introduces a Flush() method to the AsyncWriter class, providing users with more control over the asynchronous upload process. If multiple Flush() calls are made, they are queued and processed sequentially. Each call returns a future that will be satisfied only when that specific flush operation (and all preceding ones) have been acknowledged by the service.

For unbuffered (StartUnbufferedUpload) and appendable (StartAppendableObjectUpload) uploads, this is implemented using a promise queue to serialize flush operations, ensuring thread safety for concurrent use. For buffered uploads (StartBufferedUpload), the Flush() method currently forwards to Write(). A more complete implementation that forces the buffer to be sent and handles concurrent calls correctly is planned for a follow-up PR.


This change is Reviewable

@shubham-up-47 shubham-up-47 requested review from a team as code owners September 18, 2025 12:38
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Sep 18, 2025
@shubham-up-47
Copy link
Contributor Author

/gcbrun

@codecov
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 98.50746% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.05%. Comparing base (3962f01) to head (fea5648).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...oud/storage/tests/async_client_integration_test.cc 93.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #15555    +/-   ##
========================================
  Coverage   93.04%   93.05%            
========================================
  Files        2408     2408            
  Lines      220145   220278   +133     
========================================
+ Hits       204829   204970   +141     
+ Misses      15316    15308     -8     

☔ 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.

Copy link
Contributor

@bajajneha27 bajajneha27 left a comment

Choose a reason for hiding this comment

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

Can we also add an integration test for explicit flush?

@shubham-up-47
Copy link
Contributor Author

Can we also add an integration test for explicit flush?

Done.

Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

@scotthart reviewed 1 of 3 files at r1, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @bajajneha27)

@shubham-up-47 shubham-up-47 merged commit 5a88176 into googleapis:main Sep 23, 2025
67 of 72 checks passed
@shubham-up-47 shubham-up-47 deleted the add_explicit_way_to_flush branch September 23, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants