[
https://issues.apache.org/jira/browse/HDDS-17442?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17442:
------------------------------
Description:
h3. Mechanism
{{OzoneContainer}} creates one fixed size read executor
({{hdds.datanode.read.chunk.threads.per.volume}} threads per volume, 10 by
default, unbounded queue) and passes it to both {{XceiverServerGrpc}} and
{{XceiverServerDomainSocket}}. The {{Receiver}} thread of each domain socket
connection reads requests and submits one {{Receiver.ProcessRequestTask}} per
request to that executor, with no limit on how many tasks of one connection are
queued or running.
Each task dispatches its request and then calls {{Receiver.sendResponse}},
which writes the response while holding a per connection lock. A write on a
connection whose responses are not being read waits until the socket send
timeout ({{ozone.client.write.timeout}}, 30s by default). Meanwhile every other
task of the same connection that a pool thread picks up waits on the lock.
The tasks of one connection can therefore hold every thread of the executor,
and gRPC requests and the requests of other domain socket connections wait in
the queue behind them.
h3. Trigger
The executor holds at least as many tasks of one domain socket connection as it
has threads while response writes on that connection wait for the reader (for
example a paused client). Short-circuit read must be enabled on the datanode
({{ozone.client.read.short-circuit}}, off by default).
h3. Impact
* While that state lasts, requests to the datanode's gRPC container server,
which runs on the same executor, and short-circuit GetBlock requests of other
connections on that datanode get no response, so their clients time out.
* The executor is released when the connection is closed or its responses are
read. Only the datanode that holds the connection is affected.
* With the default executor size I saw the tasks of one such connection hold
all 10 threads (one in the write, nine waiting on the lock) and a gRPC GetBlock
on the same datanode get no response within 5s until the connection was closed
(one datanode MiniOzoneCluster, macOS). Not run on Linux, where the socket
buffer sizes differ, or in secure mode.
h3. Reproduction
No standalone reproduction is attached to this issue. The regression test in
the attached patch covers it and fails on unmodified source:
{{TestXceiverServerDomainSocket.testUnreadResponsesDoNotDelayOtherConnections}}
leaves the responses of one connection unread, then requires that a GetBlock on
a second connection is answered and that the first connection receives both
responses, in order, once it reads them. On unmodified source (with the suite's
one thread executor) the GetBlock on the second connection gets no response
within the 5s client read timeout and the test fails with
{{SocketTimeoutException: read(2) error: Resource temporarily unavailable}}.
h3. Patch
[^CR-4-process-domain-socket-requests-on-connection-thread.patch], against
69d77beff60b9106e2517c36a7a1b2cc726af568. It also applies to master at
1acf315d546.
{{Receiver.run}} runs each request on the connection's own thread instead of
submitting it to the shared executor. A response write that waits for its
reader then holds only that connection's thread, and gRPC requests and other
connections never queue behind it. {{Receiver}} and
{{XceiverServerDomainSocket}} no longer take the executor, and
{{OzoneContainer}} no longer passes it. The gRPC server is unchanged.
Behaviour change for clients: the requests of one connection are now processed
one at a time in arrival order. Their responses were already written one at a
time, but before the change up to pool size requests of one connection could be
dispatched in parallel, so concurrent GetBlock requests that share one
{{XceiverClientShortCircuit}} are now serialized on the datanode. A client
keeps one such connection per pipeline and sends the GetBlock requests of all
its block streams on it. GetBlock only looks up the block metadata and opens
the block file, and the block data is then read locally through the received
file descriptor, so I expect the effect to be small, but I did not measure it.
The number of domain socket requests processed at once is now bounded by the
number of connections ({{maxXceiverCount}}, five times the pool size) instead
of the pool size. The local op in queue latency metric
({{localInQueueLatencyNs<op>}}) of domain socket requests now reads about 0,
since there is no queue. There is no configuration or protocol change.
Alternatives considered: a cap on the requests one connection may have on the
shared executor keeps parallelism per connection, but several such connections
still hold the whole executor and it needs a new limit; a separate executor for
the domain socket server protects gRPC but not the other domain socket clients;
dispatching each request on the shared executor and writing its response on the
connection's thread keeps the executor's bound on concurrent work, but it
serializes the requests of a connection just the same, adds a handoff, keeps
the executor wiring, and keeps domain socket latency tied to the gRPC queue.
With the patch {{TestXceiverServerDomainSocket}} (15 tests),
{{TestXceiverClientManagerSC}}, {{TestLocalChunkInputStream}},
{{TestDomainSocketFactory}}, {{TestOzoneContainer}} and {{TestKeyValueHandler}}
pass, and checkstyle on {{hadoop-hdds/container-service}} and
{{hadoop-ozone/integration-test}} is clean.
Overlap with other open patches: this patch removes the read executor argument
of the {{XceiverServerDomainSocket}} constructor, which the new tests in the
patches of HDDS-17436, HDDS-17437 and HDDS-17441 pass, and it conflicts
textually with the patches of HDDS-17436 (test imports) and HDDS-17437 (the
{{Receiver.create}} call in {{XceiverServerDomainSocket.run}}). Whichever lands
second drops that argument or rebases.
Found by code review of the datanode short-circuit read paths, as part of the
TLA+ verification effort 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 (introduced the domain socket GetBlock channel),
HDDS-16918 (datanode logging of domain socket failures). The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
was:
h3. Mechanism
{{OzoneContainer}} creates one fixed size read executor
({{hdds.datanode.read.chunk.threads.per.volume}} threads per volume, 10 by
default, unbounded queue) and passes it to both {{XceiverServerGrpc}} and
{{XceiverServerDomainSocket}}. The {{Receiver}} thread of each domain socket
connection reads requests and submits one {{Receiver.ProcessRequestTask}} per
request to that executor, with no limit on how many tasks of one connection are
queued or running.
Each task dispatches its request and then calls {{Receiver.sendResponse}},
which writes the response while holding a per connection lock. A write on a
connection whose responses are not being read waits until the socket send
timeout ({{ozone.client.write.timeout}}, 30s by default). Meanwhile every other
task of the same connection that a pool thread picks up waits on the lock.
The tasks of one connection can therefore hold every thread of the executor,
and gRPC requests and the requests of other domain socket connections wait in
the queue behind them.
h3. Trigger
The executor holds at least as many tasks of one domain socket connection as it
has threads while response writes on that connection wait for the reader (for
example a paused client). Short-circuit read must be enabled on the datanode
({{ozone.client.read.short-circuit}}, off by default).
h3. Impact
* While that state lasts, requests to the datanode's gRPC container server,
which runs on the same executor, and short-circuit GetBlock requests of other
connections on that datanode get no response, so their clients time out.
* The executor is released when the connection is closed or its responses are
read. Only the datanode that holds the connection is affected.
* With the default executor size I saw the tasks of one such connection hold
all 10 threads (one in the write, nine waiting on the lock) and a gRPC GetBlock
on the same datanode get no response within 5s until the connection was closed
(one datanode MiniOzoneCluster, macOS). Not run on Linux, where the socket
buffer sizes differ, or in secure mode.
h3. Reproduction
No standalone reproduction is attached to this issue. The regression test in
the attached patch covers it and fails on unmodified source:
{{TestXceiverServerDomainSocket.testUnreadResponsesDoNotDelayOtherConnections}}
leaves the responses of one connection unread, then requires that a GetBlock on
a second connection is answered and that the first connection receives both
responses, in order, once it reads them. On unmodified source (with the suite's
one thread executor) the GetBlock on the second connection gets no response
within the 5s client read timeout and the test fails with
{{SocketTimeoutException: read(2) error: Resource temporarily unavailable}}.
h3. Patch
[^CR-4-process-domain-socket-requests-on-connection-thread.patch], against
69d77beff60b9106e2517c36a7a1b2cc726af568. It also applies to master at
1acf315d546.
{{Receiver.run}} runs each request on the connection's own thread instead of
submitting it to the shared executor. A response write that waits for its
reader then holds only that connection's thread, and gRPC requests and other
connections never queue behind it. {{Receiver}} and
{{XceiverServerDomainSocket}} no longer take the executor, and
{{OzoneContainer}} no longer passes it. The gRPC server is unchanged.
Behaviour change for clients: the requests of one connection are now processed
one at a time in arrival order. Their responses were already written one at a
time, but before the change up to pool size requests of one connection could be
dispatched in parallel, so concurrent GetBlock requests that share one
{{XceiverClientShortCircuit}} are now serialized on the datanode. I expect no
visible throughput change, because only GetBlock uses this connection, one per
block stream, and the block data is then read locally through the received file
descriptor, but I did not measure it. The number of domain socket requests
processed at once is now bounded by the number of connections
({{maxXceiverCount}}, five times the pool size) instead of the pool size. The
local op in queue latency metric ({{localInQueueLatencyNs<op>}}) of domain
socket requests now reads about 0, since there is no queue. There is no
configuration or protocol change.
Alternatives considered: a cap on the requests one connection may have on the
shared executor keeps parallelism per connection, but several such connections
still hold the whole executor and it needs a new limit; a separate executor for
the domain socket server protects gRPC but not the other domain socket clients;
dispatching each request on the shared executor and writing its response on the
connection's thread keeps the executor's bound on concurrent work, but it
serializes the requests of a connection just the same, adds a handoff, keeps
the executor wiring, and keeps domain socket latency tied to the gRPC queue.
With the patch {{TestXceiverServerDomainSocket}} (15 tests),
{{TestXceiverClientManagerSC}}, {{TestLocalChunkInputStream}},
{{TestDomainSocketFactory}}, {{TestOzoneContainer}} and {{TestKeyValueHandler}}
pass, and checkstyle on {{hadoop-hdds/container-service}} and
{{hadoop-ozone/integration-test}} is clean.
Overlap with other open patches: this patch removes the read executor argument
of the {{XceiverServerDomainSocket}} constructor, which the new tests in the
patches of HDDS-17436, HDDS-17437 and HDDS-17441 pass, and it conflicts
textually with the patches of HDDS-17436 (test imports) and HDDS-17437 (the
{{Receiver.create}} call in {{XceiverServerDomainSocket.run}}). Whichever lands
second drops that argument or rebases.
Found by code review of the datanode short-circuit read paths, as part of the
TLA+ verification effort 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 (introduced the domain socket GetBlock channel),
HDDS-16918 (datanode logging of domain socket failures). The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
> Domain socket requests run on the read executor shared with gRPC, where a
> response write that waits for its reader holds a thread
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17442
> URL: https://issues.apache.org/jira/browse/HDDS-17442
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Major
> Labels: pull-request-available
> Attachments:
> CR-4-process-domain-socket-requests-on-connection-thread.patch
>
>
> h3. Mechanism
> {{OzoneContainer}} creates one fixed size read executor
> ({{hdds.datanode.read.chunk.threads.per.volume}} threads per volume, 10 by
> default, unbounded queue) and passes it to both {{XceiverServerGrpc}} and
> {{XceiverServerDomainSocket}}. The {{Receiver}} thread of each domain socket
> connection reads requests and submits one {{Receiver.ProcessRequestTask}} per
> request to that executor, with no limit on how many tasks of one connection
> are queued or running.
> Each task dispatches its request and then calls {{Receiver.sendResponse}},
> which writes the response while holding a per connection lock. A write on a
> connection whose responses are not being read waits until the socket send
> timeout ({{ozone.client.write.timeout}}, 30s by default). Meanwhile every
> other task of the same connection that a pool thread picks up waits on the
> lock.
> The tasks of one connection can therefore hold every thread of the executor,
> and gRPC requests and the requests of other domain socket connections wait in
> the queue behind them.
> h3. Trigger
> The executor holds at least as many tasks of one domain socket connection as
> it has threads while response writes on that connection wait for the reader
> (for example a paused client). Short-circuit read must be enabled on the
> datanode ({{ozone.client.read.short-circuit}}, off by default).
> h3. Impact
> * While that state lasts, requests to the datanode's gRPC container server,
> which runs on the same executor, and short-circuit GetBlock requests of other
> connections on that datanode get no response, so their clients time out.
> * The executor is released when the connection is closed or its responses are
> read. Only the datanode that holds the connection is affected.
> * With the default executor size I saw the tasks of one such connection hold
> all 10 threads (one in the write, nine waiting on the lock) and a gRPC
> GetBlock on the same datanode get no response within 5s until the connection
> was closed (one datanode MiniOzoneCluster, macOS). Not run on Linux, where
> the socket buffer sizes differ, or in secure mode.
> h3. Reproduction
> No standalone reproduction is attached to this issue. The regression test in
> the attached patch covers it and fails on unmodified source:
> {{TestXceiverServerDomainSocket.testUnreadResponsesDoNotDelayOtherConnections}}
> leaves the responses of one connection unread, then requires that a GetBlock
> on a second connection is answered and that the first connection receives
> both responses, in order, once it reads them. On unmodified source (with the
> suite's one thread executor) the GetBlock on the second connection gets no
> response within the 5s client read timeout and the test fails with
> {{SocketTimeoutException: read(2) error: Resource temporarily unavailable}}.
> h3. Patch
> [^CR-4-process-domain-socket-requests-on-connection-thread.patch], against
> 69d77beff60b9106e2517c36a7a1b2cc726af568. It also applies to master at
> 1acf315d546.
> {{Receiver.run}} runs each request on the connection's own thread instead of
> submitting it to the shared executor. A response write that waits for its
> reader then holds only that connection's thread, and gRPC requests and other
> connections never queue behind it. {{Receiver}} and
> {{XceiverServerDomainSocket}} no longer take the executor, and
> {{OzoneContainer}} no longer passes it. The gRPC server is unchanged.
> Behaviour change for clients: the requests of one connection are now
> processed one at a time in arrival order. Their responses were already
> written one at a time, but before the change up to pool size requests of one
> connection could be dispatched in parallel, so concurrent GetBlock requests
> that share one {{XceiverClientShortCircuit}} are now serialized on the
> datanode. A client keeps one such connection per pipeline and sends the
> GetBlock requests of all its block streams on it. GetBlock only looks up the
> block metadata and opens the block file, and the block data is then read
> locally through the received file descriptor, so I expect the effect to be
> small, but I did not measure it. The number of domain socket requests
> processed at once is now bounded by the number of connections
> ({{maxXceiverCount}}, five times the pool size) instead of the pool size. The
> local op in queue latency metric ({{localInQueueLatencyNs<op>}}) of domain
> socket requests now reads about 0, since there is no queue. There is no
> configuration or protocol change.
> Alternatives considered: a cap on the requests one connection may have on the
> shared executor keeps parallelism per connection, but several such
> connections still hold the whole executor and it needs a new limit; a
> separate executor for the domain socket server protects gRPC but not the
> other domain socket clients; dispatching each request on the shared executor
> and writing its response on the connection's thread keeps the executor's
> bound on concurrent work, but it serializes the requests of a connection just
> the same, adds a handoff, keeps the executor wiring, and keeps domain socket
> latency tied to the gRPC queue.
> With the patch {{TestXceiverServerDomainSocket}} (15 tests),
> {{TestXceiverClientManagerSC}}, {{TestLocalChunkInputStream}},
> {{TestDomainSocketFactory}}, {{TestOzoneContainer}} and
> {{TestKeyValueHandler}} pass, and checkstyle on
> {{hadoop-hdds/container-service}} and {{hadoop-ozone/integration-test}} is
> clean.
> Overlap with other open patches: this patch removes the read executor
> argument of the {{XceiverServerDomainSocket}} constructor, which the new
> tests in the patches of HDDS-17436, HDDS-17437 and HDDS-17441 pass, and it
> conflicts textually with the patches of HDDS-17436 (test imports) and
> HDDS-17437 (the {{Receiver.create}} call in
> {{XceiverServerDomainSocket.run}}). Whichever lands second drops that
> argument or rebases.
> Found by code review of the datanode short-circuit read paths, as part of the
> TLA+ verification effort 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 (introduced the domain socket GetBlock channel),
> HDDS-16918 (datanode logging of domain socket failures). 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]
