Chunk vectored socket I/O at IOV_MAX instead of rejecting large iovec spans - #885
Open
majunze2001 wants to merge 1 commit into
Open
majunze2001 wants to merge 1 commit into
majunze2001 wants to merge 1 commit into
Conversation
… spans. WriteVExact/ReadVExact returned InvalidArgument for spans longer than IOV_MAX (1024). Per-destination-unit resharding (1eb2414) resolves a single block into thousands of small chunks, and block_transport.cc / socket_transport_adapter.cc pass the whole list in one call, so those transfers failed outright. IOV_MAX bounds one writev/readv call, not the byte stream: loop over IOV_MAX-sized sub-spans, relying on SendV/RecvV being exact within each sub-span and on TCP ordering across them. Rewrite socket_util_test.cc so it builds in this tree (it depended on //tpu_sync/transport/peregrine/src/util, which needs the absent //third_party/xxhash) and add cases at, above and far above IOV_MAX. Claude-Session: https://claude.ai/code/session_012cbF1oLqDSJhgcQS3BBhiy Signed-off-by: Jeff Ma <jeffjma@umich.edu>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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
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.
Summary
peregrine::WriteVExact/ReadVExactreturnedInvalidArgumentError("#iovs=N")for any span longer thanIOV_MAX(1024 onLinux). Sharded / hybrid KV cache resharding now resolves a single block into
thousands of small, non-contiguous byte spans, and the block transport hands
that whole list to these helpers in one call, so such transfers failed
outright.
This change makes the helpers loop over the span in
IOV_MAX-sized pieces.IOV_MAXbounds onewritev(2)/readv(2)call, not the logical bytestream. The test is rewritten so it builds in the OSS tree and gains cases at,
above, and far above the limit.
Root cause
tpu_sync/transport/peregrine/src/api/socket_util.hforwardto
TcpSocketUtil::SendV/RecvVonly when1 <= n <= IOV_MAX, otherwisethey error without touching the socket.
KVCacheManagerBase::GetBlockChunks(plan-driven path) emits one chunk perstrided unit of every matching schedule entry. PR Route byte spans per destination unit for sharded KV cache resharding. #829 /
1eb2414("Routebyte spans per destination unit for sharded KV cache resharding") made the
planner produce per-destination-unit spans, so one block can resolve to
many thousands of chunks. The plan-less path does the same for sparse pool
regions via
ComputePoolBlockCopyExtents.block_transport.ccandsocket_transport_adapter.ccconvert those chunksto one iovec each and call the helpers once per block with no cap.
(
raw_buffer_transport.cccaps its batches atIOV_MAXitself and is notaffected.)
InvalidArgument; it logs "Write payload failed", shuts the socket and dropsthe send. Receiver side returns the same error and drops the connection.
Deterministic hard failure, no corruption.
Fix
socket_util.h: whenn > IOV_MAX, callSendV/RecvVon consecutiveIOV_MAX-sized sub-spans until the span is exhausted, returning the firstnon-OK status. Correctness rests on two facts:
TcpSocketUtil::SendV/RecvValready loop until every byte of theirsub-span has moved (they handle partial
writev/readvreturns).not agree.
Each sub-span still satisfies the
DCHECK_LE(iovecs.size(), IOV_MAX)insidethe inner helpers. The
n == 0case still returnsInvalidArgument.Tests
socket_util_test.ccis rewritten://tpu_sync/transport/peregrine/src/util(
RandomNonZero), which depends on//third_party/xxhash. That target doesnot exist in this repository, so the test could not build here. The new
version uses only in-tree APIs (
IpAddr::Create,TcpSocket::Create, port 0plus
SelfAddrPortto discover the listening port) and a deterministic1 + i % 251byte pattern instead of RNG.ReadWriteAtIovMax(exactlyIOV_MAXon both sides),ReadWriteAboveIovMax(IOV_MAX + 1read vs2 * IOV_MAX + 3write) andReadWriteManyIovs(16 * IOV_MAX + 3vs14 * IOV_MAX + 1, above thefragment counts seen in hybrid-state resharding). All run across
IPv4/IPv6 x plain/vectored read x plain/vectored write.
api/BUILD: test deps swapinternal/util:test_utilandsrc/utilforinternal/base:ipaddrandinternal/socket:socket_util.Testing performed
With bazel 8.6.0,
--config=oss, clang 18://tpu_sync/transport/peregrine/src/api:socket_util_test: passes, 32/32cases across all parameterisations.
socket_transport_adapter_test,chunk_serializer_test,peregrine_control_service_test: pass.block_transport_testandraw_buffer_transport_testdo not build in thistree as-is (pre-existing: missing
//third_party/xxhash, and google3-onlyASSERT_OK-family macros). Built with local, uncommitted shims,block_transport_testpasses;raw_buffer_transport_testhas one flakycase (
PushBuffersTriggersOnDataReceivedWhenChunksExceedOrEqualExpected)that fails at the same rate without this change and never reaches the new
code path, since that transport caps batches at
IOV_MAX.