[SYSTEMDS-3946] Enable sending of large (>2GiB) FederatedRequests and…#2496
Open
Biranavan-Parameswaran wants to merge 1 commit into
Open
Conversation
… Responses Federated transfers previously failed for payloads above 2GiB because the single Netty frame size is bounded by a 32-bit length field, capping any request or response at Integer.MAX_VALUE bytes. This patch adds a streaming chunked codec that splits a large payload into bounded frames on the sender and reassembles them on the receiver, so the on-wire size is no longer limited by a single frame. A format detector and format encoder select the chunked path only when the payload exceeds the frame limit, leaving the existing small-message path unchanged to avoid added overhead for the common case. Adds FederatedMaxPayloadTest to exercise the boundary around the former 2GiB cap.
ywcb00
requested changes
Jun 22, 2026
ywcb00
left a comment
ywcb00
left a comment
Contributor
There was a problem hiding this comment.
Thank you very much for the PR @Biranavan-Parameswaran :)
I left some minor comments in the code. Could you please have a look at it and resolve it if you find the time. Thanks.
|
|
||
| static final byte MARKER_LEGACY = 0; | ||
| static final byte MARKER_CHUNKED = 1; | ||
| static final long STREAM_THRESHOLD = 1536L << 20; // ~1.5 GB: route below this through the legacy object codec |
Contributor
There was a problem hiding this comment.
We should use the regular encoder as long as we can, i.e., up to the largest possible message size. Can we increase this default threshold from 1.5GB to (INT_MAX - 1) bytes?
Comment on lines
+62
to
+64
| catch(ExecutionException e) { | ||
| Assert.fail("Federated transfer failed: " + e.getMessage()); | ||
| } |
Contributor
There was a problem hiding this comment.
No need for this catch as the exception is anyways caught by the catch block that is directly below. (redundant)
| import io.netty.channel.ChannelPipeline; | ||
| import io.netty.handler.codec.ByteToMessageDecoder; | ||
|
|
||
| public final class FederatedFormatDetector extends ByteToMessageDecoder { |
Contributor
There was a problem hiding this comment.
Would it be more intuitive to name this class "FederatedFormatDecoder"?
github-project-automation
Bot
moved this from In Progress
to In Review
in SystemDS PR Queue
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Federated transfers previously failed for payloads above 2GiB because the
single Netty frame size is bounded by a 32-bit length field, capping any
request or response at Integer.MAX_VALUE bytes.
This patch adds a streaming chunked codec that splits a large payload into
bounded frames on the sender and reassembles them on the receiver, so the
on-wire size is no longer limited by a single frame. A format detector and
format encoder select the chunked path only when the payload exceeds the
frame limit, leaving the existing small-message path unchanged to avoid
added overhead for the common case.
Adds FederatedMaxPayloadTest to exercise the boundary around the former
2GiB cap.