Send buffers in chunks <2GB #5134
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
This was referenced Jul 28, 2021
jakirkham
force-pushed
the
send_lt_2gb_bufs
branch
from
f9cae33 to
b698031
Compare
Works around the same OpenSSL issue seen for reading except this does so for writing. As individual frames may not be this large, this may be less of an issue. Still this is a good preventative measure to protect users.
jakirkham
force-pushed
the
send_lt_2gb_bufs
branch
from
b698031 to
63b0c79
Compare
All chunks will be of non-trivial size. If they are trivial, the loop would have already ended.
jakirkham
force-pushed
the
send_lt_2gb_bufs
branch
from
2c8939b to
445a38b
Compare
|
I suspect that this could be handled by the current max shard size functionality. We recently did this for websockets in other to keep the frame size below 8MB. |
|
Relevant PR: #5070 (I was on my phone last message) |
|
Would suggest merging so that both sides of the OpenSSL issue are fixed. Especially in the upcoming release ( dask/community#173 ). If someone wants to come along and improve on this work, that would be great, but cannot personally promise to do that (particularly not in the near future). |
mrocklin
added a commit
to mrocklin/distributed
that referenced
this pull request
Jul 29, 2021
Supercedes dask#5134 Copying over the summary of that PR Works around the OpenSSL 1.0.2 bug demonstrated in issue ( dask#4538 ), except unlike PR ( dask#5115 ) which did this for reading, this does the same thing for writing. The error may be less likely to show up in the write path (as frames may simply be smaller than this limit). Still it seems like a good idea to protect against OverflowErrors from OpenSSL
3 tasks
|
I'm thinking of the following: #5141 |
|
Great, feel free to close this then :) |
|
Closing in favor of #5141 |
jrbourbeau
pushed a commit
that referenced
this pull request
Jul 29, 2021
Supercedes #5134 Copying over the summary of that PR Works around the OpenSSL 1.0.2 bug demonstrated in issue ( #4538 ), except unlike PR ( #5115 ) which did this for reading, this does the same thing for writing. The error may be less likely to show up in the write path (as frames may simply be smaller than this limit). Still it seems like a good idea to protect against OverflowErrors from OpenSSL
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.
Works around the OpenSSL 1.0.2 bug demonstrated in issue ( #4538 ), except unlike PR ( #5115 ) which did this for reading, this does the same thing for writing. The error may be less likely to show up in the write path (as frames may simply be smaller than this limit). Still it seems like a good idea to protect against
OverflowErrors from OpenSSL.black distributed/flake8 distributed/isort distributed