feat: update Storage.createFrom(BlobInfo, Path) to have 150% higher throughput #2059
Conversation
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
BenWhitehead added do not merge 
product-auto-label bot added size: xl 
Collaborator Author
| ITRetryConformanceTests are currently failing due to googleapis/storage-testbench#510 |
BenWhitehead force-pushed the put-file branch 4 times, most recently from d29afd4 to 0a6f7c7 Compare 
When uploading a file where we are able to rewind to an arbitrary offset, we can be more optimistic in the way we send requests to GCS. Add new code middleware to allow PUTing an entire file to GCS in a single request, and using query resumable session to recover from the specific offset in the case of retryable error. ## TODO 1. Add failure scenario: send more bytes than are specified in content-range header
frankyn reviewed Jun 29, 2023
google-cloud-storage/src/main/java/com/google/cloud/storage/ResumableSession.java Outdated Show resolved Hide resolved
...loud-storage/src/main/java/com/google/cloud/storage/JsonResumableSessionFailureScenario.java Show resolved Hide resolved
google-cloud-storage/src/test/java/com/google/cloud/storage/FakeHttpServer.java Show resolved Hide resolved
...-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionPutTaskTest.java Show resolved Hide resolved
...-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionPutTaskTest.java Show resolved Hide resolved
...-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionPutTaskTest.java Show resolved Hide resolved
...-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionPutTaskTest.java Show resolved Hide resolved
...-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionPutTaskTest.java Show resolved Hide resolved
...-cloud-storage/src/test/java/com/google/cloud/storage/ITJsonResumableSessionPutTaskTest.java Outdated Show resolved Hide resolved
BenWhitehead changed the title BenWhitehead changed the title frankyn approved these changes Jul 10, 2023
Contributor
frankyn left a comment
frankyn left a comment There was a problem hiding this comment.
Had one comment on a todo; otherwise lgtm
| | ||
| @ParametersAreNonnullByDefault | ||
| enum JsonResumableSessionFailureScenario { | ||
| // TODO: send more bytes than are in the Content-Range header |
Contributor
There was a problem hiding this comment.
wouldn't the server just stop reading bytes after content-range is satisfied? Guessing this would be more of a dataloss scenario client side?
Collaborator Author
There was a problem hiding this comment.
I spent some time thinking about this, and this would actually be a client side bug. Since the client computes both Content-Range and Content-Length if those don't sync up, we have a bug so not a server response failure we need to handle. I'll remove this TODO in the follow up PR I have queued up on my workstation to follow this one.
Contributor
There was a problem hiding this comment.
Client side issue makes sense; if anything tests would serve to protect client changes that introduce such a bug.
BenWhitehead removed the do not merge 
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
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.
When uploading a file where we are able to rewind to an arbitrary offset, we can be more optimistic in the way we send requests to GCS.
Add new code middleware to allow PUTing an entire file to GCS in a single request, and using query resumable session to recover from the specific offset in the case of retryable error.
Benchmark Results
Methodology
Generate a random file on disk of size
128KiB..2GiBfrom/dev/urandom, then upload the generated file usingStorage.createFrom(BlobInfo, Path).Perform each 4096 times.
Run on a c2-standard-60 instance is us-central1 against a regional bucket located in us-central1.
Results
The following summary of throughput in MiB/s as observed between the existing implementation, and the new implementation proposed in this PR.
Comparison
When comparing the new implementation to the existing implementation we get the following improvement to throughput (higher is better):