feat(gax): implement baseline Callable and Future for resumable uploads - #14241
feat(gax): implement baseline Callable and Future for resumable uploads#14241whowes wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
554caca to
b8fc38f
Compare
|
/gemini review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
b8fc38f to
234e79f
Compare
234e79f to
6845669
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the concrete implementation of ResumableUploadCallable and ResumableUploadFuture (ResumableUploadCallableImpl and ResumableUploadFutureImpl) to coordinate resumable upload sessions and stream chunks asynchronously, along with comprehensive unit tests. The feedback highlights a potential issue where performing blocking I/O (ByteStreams.read) inside asynchronous future callbacks could lead to thread starvation or deadlocks if a limited executor is used, suggesting either documenting executor requirements or offloading the blocking read.
| byte[] buffer = new byte[chunkSize]; | ||
| int bytesRead; | ||
| try { | ||
| bytesRead = ByteStreams.read(payload, buffer, 0, chunkSize); |
There was a problem hiding this comment.
Performing blocking I/O (ByteStreams.read) inside asynchronous future callbacks (which run on the provided executor) can lead to thread starvation or deadlocks if the executor is a direct executor or a limited thread pool (such as gRPC network threads). Consider documenting that the executor passed to the callable must be a dedicated thread pool suitable for blocking I/O operations, or offloading the blocking read to a dedicated I/O executor.
|
|
1 participant
This implementation supports the happy path only; retries, recovery, and progress tracking will be added in subsequent phases.