This repository has been archived by the owner on Nov 8, 2024. It is now read-only.
Experiment: Parallel Multipart Upload #22
Closed
Conversation
This file contains 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
Previously, a MPU would succeed in writing all the chunks and composing into the desired destination. The chunkNames argument however included the final destination name (as it is used for the "first" chunk); when deferredCleanup would finally complete the destination file would also be deleted. This corrects the behavior by excluding the first chunk name in deferredCleanup via a filter on the argument. This also includes a fix for a possible OutOfMemoryError condition being raised with blobs that have a really large final chunk. We avoid the issue by avoiding a call to readChunk (and a Arrays.copyOfRange on the chunk) for the final chunk. Instead, we pass the remaining InputStream to the storage client, with the unfortunate side effect of calling a deprecated method. Per the docs, the storage#create method is deprecated because it is not retryable. That means we may have failed uploads where the first 31 chunks succeed, and the last fails without retry; that highlights the importance of the tuning option and the log messaging alerting of the issue. Also includes: * an integration test to mimic storage facet's write temp and move behavior * new tests on the uploader with larger blobs * increases the maxConnectionsPerRoute to equal maxTotalConnections
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This pull request captures an experiment with using parallel multipart upload and Google Cloud Storage Composite Objects.
This change set includes a new client,
MultipartUploaderthat encapsulates the splitting of an incoming stream, uploading them in parallel, and finally issuing a compose request to combine them at close.Some observations:
byte[]rather than anInputStream, and it does so for an interesting reason. Thebyte[]variant automatically can retry on some failure cases and is recommended, the InputStream variant does not, since the stream can only be used once.chunkSizeto allow deployers to customize based on workloads (some repository formats have larger assets than others). If the chunkSize is too low, you could easily run into a circumstance with 31 chunks of a small size and 1 giant chunk that we have to wait for.Does it perform better or worse than single thread? I setup a single NXRM instance with the plugin on a n1-highmem-4 GCP instance, and used another GCP instance in the same VPC to generate a diverse workload. I ran the same workload with the current master build (using single threaded uploads).
The results? Higher CPU utilization and lower overall network I/O:
I don't believe this to be suitable approach for NXRM blobstores at this time.
This was a valuable experiment. I will be porting the increase to the connections per route configuration and some of the integration tests to the final product. I do not intend to merge this change set at this time; opening this PR just to capture the results. Relates to #1.