Skip to content

Conversation

@mrocklin
Copy link
Member

@mrocklin mrocklin commented Aug 1, 2021

Today we try to split up large messages in comms.
This is useful in a few situations:

  1. Websockets, which often pass frames through middleware that requires
    small messages
  2. TLS, which fails on some OpenSSL versions with frames above the size
    of an int

We correctly cut up data frames into smaller pieces to address these
issues. However we don't apply this same logic to the header frame,
which may still contain very large bytestrings. This commit adds a
workaround in protocol dumps/loads which watches for this event and
splits the header frame up if necessary.

It works, but it's not very smooth. I would prefer that in the future
we think about what a proper header should look like and ensure that it
contains no user data. In the meantime this should help.

cc @ian-r-rose @jakirkham

Today we try to split up large messages in comms.
This is useful in a few situations:

1.  Websockets, which often pass frames through middleware that requires
    small messages
2.  TLS, which fails on some OpenSSL versions with frames above the size
    of an int

We correctly cut up data frames into smaller pieces to address these
issues.  However we don't apply this same logic to the header frame,
which may still contain very large bytestrings.  This commit adds a
workaround in protocol dumps/loads which watches for this event and
splits the header frame up if necessary.

It works, but it's not very smooth.  I would prefer that in the future
we think about what a proper header should look like and ensure that it
contains no user data.  In the meantime this should help.
@mrocklin
Copy link
Member Author

mrocklin commented Aug 2, 2021

Also, I don't recommend merging this yet. I'd love for some more general solution, like #5150 to go in instead. In general we should minimize the number of changes to the protocol. This causes harder breaks when mixing Dask versions.

@mrocklin mrocklin requested a review from fjetter as a code owner January 23, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant