Add documentation to concurrent multipart upload utility methods #128821
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
In the hope to make more sense of how the flux of byte buffer works. Also remove the synchronization on the input stream. Relates ES-11815
tlrx
added
>non-issue
:Distributed Coordination/Snapshot/Restore
elasticsearchmachine
added
the
Team:Distributed Coordination
Collaborator
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
henningandersen
approved these changes
Jun 3, 2025
| // So this flux instantiates 128 ByteBuffer objects of DEFAULT_UPLOAD_BUFFERS_SIZE bytes in heap every time the NettyOutbound in | ||
| // the Azure's Netty event loop requests byte buffers to write to the network channel. That represents 128 * 64kb = 8 mb per | ||
| // flux. The creation of the ByteBuffer objects are forked to the repository_azure thread pool, which has a maximum of 15 | ||
| // threads (most of the time, can be less than that for nodes with less than 750mb). It means that max. 15 * 8 = 120mb bytes are |
Contributor
There was a problem hiding this comment.
Suggested change
| // threads (most of the time, can be less than that for nodes with less than 750mb). It means that max. 15 * 8 = 120mb bytes are | |
| // threads (most of the time, can be less than that for nodes with less than 750mb heap). It means that max. 15 * 8 = 120mb bytes are |
?
| * | ||
| * @param stream the input stream that needs to be converted | ||
| * @param length the expected length in bytes of the input stream | ||
| * @param byteBufferSize the size of the ByteBuffer to be created |
Contributor
There was a problem hiding this comment.
Suggested change
| * @param byteBufferSize the size of the ByteBuffer to be created | |
| * @param byteBufferSize the size of the ByteBuffers to be created |
| } | ||
| return ByteBuffer.wrap(buffer); | ||
| })).doOnComplete(() -> { | ||
| }), 0 /* no prefetching needed for concatMap as the MonoSendMany subscriber requests 128 elements */).doOnComplete(() -> { |
Contributor
There was a problem hiding this comment.
I am not entirely sure I understand this comment. Is it an important thing or a nit? It seems like we'd generate 128 elements regardless of this being 0 or the default? Is there a difference in when?
Member
Author
There was a problem hiding this comment.
It's really a nit; I've been puzzled about the byte buffers being fetched by 32 instead of the 128 from the initial comment. This prefetching do not have much impact in our case I think.
tlrx
added
the
auto-merge-without-approval
tlrx
deleted the
2025/06/03/add-note-on-concurrent-multipart-uploads-methods
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
In the hope to make more sense of how the flux of byte buffer works. Also remove the synchronization on the input stream.
Relates ES-11815