Repository navigation
Conversation
…th gRPC, where a response write that waits for its reader holds a thread Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@smengcl , I'm afraid this approach could impact the short-circuit performance with a noticeable degradation. Let me check it with some test. |
|
Random read and vector search benchmark After the patch |
Generated-by: Claude Code (Opus 5.5)
What changes were proposed in this pull request?
OzoneContainercreates one fixed size read executor (hdds.datanode.read.chunk.threads.per.volumethreads per volume) and passes it to bothXceiverServerGrpcandXceiverServerDomainSocket. TheReceiverof each domain socket connection submits oneProcessRequestTaskper request to that executor, with no limit per connection. Each task dispatches its request and then writes the response inReceiver.sendResponsewhile holding a per connection lock, so a write that waits for its reader (up to the socket send timeout) holds a pool thread, and every other task of that 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 other domain socket connections queue behind them.This change makes
Receiver.runprocess each request on the connection's own thread instead of the shared executor, so a write that waits for its reader holds only that connection's thread.ReceiverandXceiverServerDomainSocketno longer take the executor, andOzoneContainerno longer passes it. The gRPC server is unchanged. There is no configuration or protocol change.Behaviour change: requests of one domain socket connection are now processed one at a time in arrival order (responses were already written one at a time). A client keeps one such connection per pipeline and sends the GetBlock requests of all its block streams on it, so concurrent GetBlocks from one client to its local datanode are now dispatched one after another. GetBlock only looks up the block metadata and opens the block file, and block data is read locally through the received descriptor, so the effect should be small; it was not measured. The local op in queue latency metric of domain socket requests now reads about 0.
Alternatives considered: a cap on one connection's tasks in the shared executor (several connections can still hold the whole executor, and it needs a new limit); a separate executor for the domain socket server (protects gRPC but not other domain socket clients); dispatching on the shared executor and writing the response on the connection's thread (serializes per connection just the same, with more wiring).
Overlap with other open changes: this removes the executor argument of the
XceiverServerDomainSocketconstructor, which the new tests of HDDS-17436 and HDDS-17441 pass, and both conflict textually with it in the test imports. Whichever lands second drops the argument or rebases.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-17442
How was this patch tested?
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. Without the change the second GetBlock gets no response within the 5 s read timeout (SocketTimeoutException).TestXceiverServerDomainSocket(15 tests),TestXceiverClientManagerSC,TestLocalChunkInputStream,TestDomainSocketFactory,TestOzoneContainerandTestKeyValueHandlerpass locally on macOS; checkstyle is clean. Not run on Linux.