[SYSTEMDS-3946] Enable sending of large (>2GiB) FederatedRequests and… - #2496
Conversation
ywcb00
left a comment
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 |
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?
There was a problem hiding this comment.
Good suggestion, but INT_MAX-1 is actually unsafe here. Routing uses a size estimate, not the exact wire size, and the ObjectEncoder overflows its Integer.MAX_VALUE ByteBuf once serialization framing is added. I measured that cliff at about 1.990 GiB, so an estimate just under INT_MAX can still overflow on the wire. I set the threshold to 2000L << 20 (about 1.953 GiB), roughly 40 MB under the cliff, so the regular encoder is used as high as we safely can.
Note the routing also changed since your review. Responses now route by lineage cacheability instead of size, so STREAM_THRESHOLD is now the size guard on the object encoder path rather than the main router.
f5073a4 to
ec0538b
Compare
b5e4165 to
ec0538b
Compare
|
Thanks for the review @ywcb00. All three comments are addressed, and I also ran experiments to back the design choices. Summary of what changed since the reviewed commit. Comments
Follow up work
|
661bf3f to
0e2fc2a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2496 +/- ##
============================================
- Coverage 71.60% 71.59% -0.01%
- Complexity 50259 50390 +131
============================================
Files 1623 1631 +8
Lines 194314 194856 +542
Branches 37965 38025 +60
============================================
+ Hits 139130 139506 +376
- Misses 44277 44413 +136
- Partials 10907 10937 +30 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ywcb00
left a comment
There was a problem hiding this comment.
Thank you very much for this contribution. :)
The code looks very good. I only added some minor comments regarding variable naming.
A general aspect to consider: Do you think it is possible to make the FederatedChunkCodecTest even faster in terms of runtime needed for the test while testing the same functionality?
All the best,
David
|
|
||
| static final int HEADER_LEN = 5; | ||
| static final int DEFAULT_CHUNK_SIZE = 1 << 22; | ||
| static final int QUEUE_DEPTH = 16; |
There was a problem hiding this comment.
How did you decide on this number here? Was it chosen arbitrarily or are there certain factors/reasons behind this decision?
There was a problem hiding this comment.
It's randomly chosen because the queue never fills up locally to test it.
Happy to adjust or make it configurable.
… 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.
a9ab2a8 to
4b681ef
Compare
Thanks @ywcb00 , all comments resolved! :) Regarding |
32bb284 to
d96e845
Compare
Document the codec on methods and fail fast on an unknown frame type in the chunk decoder.
d96e845 to
bc74634
Compare
Both negative tests go through a helper, since writeInbound rethrows if the deserializer thread wins the race.
Federated transfers previously failed for payloads above 2 GiB. Netty's frame length field is a signed 32-bit integer, so a single request or response is capped at
Integer.MAX_VALUEbytes.This patch adds a streaming chunked codec. The sender splits a serialized payload into bounded frames and the receiver reassembles them, so the size of one logical message on the wire is no longer limited by a single frame.
Wire format
Every message is preceded by a one byte format marker, so the receiver knows which decoder to use:
MARKER_OBJECT_ENCODERorMARKER_CHUNKED. A chunked message then follows as a sequence of frames:FederatedChunkEncoderserializes on a pool thread and feeds frames through Netty'sChunkedWriteHandler.FederatedChunkDecoderreassembles frames into anObjectInputStreamthat the deserializer consumes as they arrive. Neither side ever materializes the whole payload as a single array, while transferring.Backpressure runs in both directions: a sender that outruns the socket and a receiver that outruns the deserializer each pause until the other side catches up, so a 4 GB message still holds only about 64 MB in memory at a time when transferring.
Routing
FederatedFormatEncoder.useObjectEncoder()uses the legacyObjectEncoderonly when the lineage cache is active and the message is a lineage cacheableFederatedResponsebelowSTREAM_THRESHOLD, which is 2000 MiB or 1.953 GiB. Thelineage cache is off by default, so in a default configuration every message takes the chunked path.
Why the legacy encoder is kept at all: the lineage serialization cache (
LineageCache.putSerializedObject) stores an entire serialized response as oneINT_MAXboundedbyte[]and only theObjectEncoderpath produces that array. Streaming never materializes it. So the legacy path survives for exactly the case that requires it and for nothing else.A size guard is still needed because that
byte[]cannot exceedINT_MAX, so a response too large for it has to stream whether or not it is cacheable.The guard is deliberately 2000 MiB rather than
INT_MAX - 1. It sees an estimate of the raw payload, but the wire adds a measured 0.4883% of framing, so a payload gated atINT_MAX - 1overflows by about 10 MB. The wire was measured to reachInteger.MAX_VALUEat a raw payload of about 1.990 GiB, and 2000 MiB sits around 38 MiB under that.Experiments
One client and one worker on the same host, timed as the client's
Total elapsed timeover an elementwise operation on a federated matrix, so the matrix crosses the codec. AMD Ryzen 5 4600H, 12 cores, 11 GB RAM, OpenJDK17.0.10 on WSL2, both JVMs at
-Xmx8g -Xms1g -Xmn256m.n = 15 for 80 KB and 8 MB, n = 3 for 512 MB and 1 GiB. Small payloads require extra runs since their effect size matches the background noise between runs. At 512 MB and 1 GiB the gap is 4x to 5x, far outside the spread, and one 1 GiB pair already costs about 6.5 minutes.
Small payloads: no measurable regression. Chunked is 2.9 % slower at 80 KB and 0.9 % at 8 MB, both within the run to run noise. The overhead is fixed per message, so it does not grow with the payload.
Large payloads. The object encoder holds the whole message as one contiguous buffer at both ends and cannot start sending before serialization finishes. The chunked codec overlaps the two and never holds more than the frame queue, so cost stays close to linear and memory stays bounded.
Different baseline. These figures supersede an earlier table in this thread, which compared against an outdated baseline.
Chunk size
4 MB (
1 << 22), from a sweep over 256K, 1M, 4M and 8M across four payloads, 48 runs. Medianfed_+in seconds, the federated instruction time, not the full round trip of the table above:Median
fed_+against chunk size, as a percentage faster than the 256 KB chunk, one line per payload. 80 KB is not drawn, it is one frame at every chunk size.4 MB wins at 512 MB and at 1074 MB and holds as the payload doubles, while 8M gives part of that back (4.3 % slower at 512 MB, 2.2 % at 1074 MB) and 256K pays too much framing overhead. 8M leads the 8 MB row only because the payload fits in one frame there, and 0.08 MB is one frame at every size, so neither row says anything about chunking. 4 MB also caps buffering in flight at 64 MB.
Above the 2 GiB cap
The range the codec exists for. One run per size, on a different macOS host with about 20 GB of usable RAM and a lighter workload, so these absolute numbers do not compare with the round trip table further up. The trend within the
table is the point:
Peak resident memory of the worker against payload size, above the 2 GiB single frame cap with a 2x payload reference line. Sampled every 0.4 s, 1 run per payload.
All correct, 0 exceptions, 0 OOM. Throughput stays flat and peak worker RSS stays at 2.2x to 3.0x the payload, which is the workload's own two copies, the received matrix plus the result, with no serialization buffer on top. The object encoder
cannot reach this range at all, it fails at about 1.99 GiB.
Known limitations
ObjectEncoderpath cannot be removed until that cache can accept a streamed response.