volume put: concurrent sha256 hash for multipart uploads #2724
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
danielnorberg
force-pushed
the
dano/volume-put-concurrent-hash
branch
from
c7cfe01 to
53187c0
Compare
|
Should probably add a test for this? Or do we already have tests that cover it? |
|
Does not look like any of the existing tests cover this. I'll add a test. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Concurrently compute sha256 hash while uploading for volume put operations of large files.
This part of the client code is maybe already too gnarly to the added complexity, but this was the smallest change I could come up with and the gains are significant for large volume puts.
Another option would be to omit the client-side sha256 digest entirely, but the complexity seems to end up being similar and then we also need server-side changes to handle the lack of
sha256_hexin theMountPutFilehandler. We also lose some integrity protections, some of which were recently added and might thus be desirable to keep.