[
https://issues.apache.org/jira/browse/HDDS-17444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gargi Jaiswal reassigned HDDS-17444:
------------------------------------
Assignee: Gargi Jaiswal
> Domain socket Receiver keeps the connection after a response send fails
> partway, so the next response follows a cut off frame
> -----------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17444
> URL: https://issues.apache.org/jira/browse/HDDS-17444
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Assignee: Gargi Jaiswal
> Priority: Major
> Attachments:
> MC-8-close-domain-socket-connection-after-failed-response-send.patch,
> TestBugMC8PartialResponseKeepsConnection.java
>
>
> h3. Mechanism
> {{Receiver.sendResponse}} writes the version, the command type, the
> length-prefixed response and, for a granted GetBlock, the file descriptor. If
> any of these writes fails, for example on the send timeout
> ({{ozone.client.write.timeout}}, 30 s by default) after part of the response
> is on the wire, its {{catch (Throwable)}} only logs "Failed to send response"
> and closes the block file. The connection stays open and the read loop keeps
> handling requests, so the next response is written right after the cut off
> one.
> The client ({{XceiverClientShortCircuit.ReceiveResponseTask}}) reads the
> length the first frame declared and fills it with the bytes that follow, then
> matches the parsed message by its clientId and callId, which are the last
> fields of the message. By code reading the same happens when the descriptor
> send fails after the response was flushed: the client then takes the first
> byte of the next frame as the descriptor byte and the stream is out of step
> by one byte (not run).
> The log and continue handling dates from HDDS-11627.
> h3. Trigger
> Responses are pending on a connection and the client does not read for the
> whole send timeout, so the datanode's socket buffer fills and a send times
> out partway. The client's reader is a dedicated thread, so this needs the
> client's receive thread to stall for that long. By default the client's own
> request timeout ({{ozone.client.read.timeout}}, 30 s) equals the datanode's
> send timeout, so the request whose response is cut has usually already failed
> on the client when the send fails; a later request completes from the stream
> only if it was sent while the receive thread alone was stalled.
> h3. Impact
> * Shown on a real {{XceiverServerDomainSocket}}: after a send timeout the
> connection stays open, and the bytes on it are the first part of a 4 MB
> response (8192 bytes on macOS with the default socket buffer) followed by the
> complete next response.
> * By code reading, the client then parses the next bytes as the rest of the
> cut-off frame. If too few bytes follow, its read blocks until
> {{ozone.client.read.timeout}} and then fails every pending request. Otherwise
> the merged bytes are parsed as one message, which usually fails to parse or
> carries a callId the client no longer waits for, and then the receive loop
> also fails every pending request. Both outcomes end in the gRPC fallback.
> Which one occurs depends on where the cut falls. In a run with the real
> {{XceiverClientShortCircuit}} (receive thread held until the datanode logged
> the failed send, three Echo requests pending, not attached), all three
> requests stayed pending until their request timers fired, and the client
> still treated the connection as open.
> * By code reading, not run: when the cut-off bytes parse as one message for
> the same block, protobuf merges the two chunk lists, and since
> {{BlockInputStream}} computes chunk positions from chunk lengths, a read
> bound to that descriptor could return bytes that do not belong at that
> offset. This was not shown. Either way, the framing on the connection can no
> longer be trusted.
> h3. Reproduction
> [^TestBugMC8PartialResponseKeepsConnection.java]
> ({{hadoop-ozone/integration-test}}) runs a real {{XceiverServerDomainSocket}}
> with {{HddsDispatcher}} and {{KeyValueHandler}}, a 1 s send timeout and the
> native libhadoop ({{-Phadoop-native-lib}}), and changes no source. It sends
> two Echo requests on one connection, the first asking for a 4 MB response,
> and reads nothing until the datanode has logged the failed send. The client's
> read then times out on the open connection, and the bytes it received are the
> first part of the first response (8192 of 4194341 bytes on macOS with the
> default socket buffer) followed by the second response, compared byte for
> byte. Echo with a large response only makes the send timeout deterministic;
> GetBlock responses and descriptors use the same write path. The test passes
> on unmodified source, so a passing test means the defect is present.
> h3. Patch
> [^MC-8-close-domain-socket-connection-after-failed-response-send.patch],
> against 69d77beff60b9106e2517c36a7a1b2cc726af568. It also applies to master
> at 1acf315d546.
> When a send fails, {{sendResponse}} now closes the peer after logging, while
> it still holds the connection's send lock, so no other response can follow
> the partial one. The read loop then ends through its normal path and
> {{XceiverServerDomainSocket.closePeer}} releases the connection once, as
> before. Responses still queued for that connection fail at once and close
> their block files. On the client, the receive loop fails on the truncated
> message and fails every pending request at once, as it does when the datanode
> closes an idle connection, and the reads fall back to gRPC. In the client run
> described under Impact, with the patch the three pending requests failed
> within a few milliseconds with {{InvalidProtocolBufferException}} (the input
> ended in the middle of a field) through the receive loop's generic {{catch}},
> and a later request failed with "... is not open". Nothing changes while
> sends succeed. A client side check cannot make a stream with a partial frame
> parseable again, so closing is the repair.
> Covered by {{testConnectionClosedAfterFailedResponse}} in the existing
> {{TestXceiverServerDomainSocket}}: without the change the client's read times
> out on the open connection ({{SocketTimeoutException}}), with it the client
> receives part of the first response and then the end of the stream, and the
> second response is not among the received bytes. This patch and the patch of
> the separate issue HDDS-17443 add the same test helpers ({{sendRequest}},
> {{getEchoRequest}}) to {{TestXceiverServerDomainSocket}}, so whichever is
> applied second needs a trivial rebase that drops the duplicate. The new test
> passes the read executor argument of the {{XceiverServerDomainSocket}}
> constructor like the existing tests; after the change in HDDS-17442, which
> removes that argument, it drops it as well. With the change
> {{TestXceiverServerDomainSocket}}, {{TestXceiverClientManagerSC}},
> {{TestLocalChunkInputStream}} and {{TestDomainSocketFactory}} (18 tests) and
> {{TestKeyValueHandler}} (43 tests) pass, checkstyle is clean, and the
> reproduction fails because the stream ends instead.
> Found by TLA+ model checking and code review of the datanode short-circuit
> read paths under HDDS-15926, on commit
> 69d77beff60b9106e2517c36a7a1b2cc726af568. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-11627 (added GetBlock on the short-circuit channel and this
> send path), HDDS-16335 (concurrent access on the client side of the domain
> socket). The attached patch is a proposal for review. Generated with Specula
> (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
