HDDS-16335. Change XceiverClientShortCircuit to support concurrent access

[echonesis]

What changes were proposed in this pull request?

XceiverClientShortCircuit did not consistently synchronize access to its connection state and other non-final fields.

This pull request uses the existing lock to protect all non-final fields and serialize connection setup, request registration and writes, and close transitions. Blocking reads, response waits, and receiver joins remain outside the lock.

Thread-safe final fields are used for response and timeout processing so they can proceed while a socket write is blocked. The client also remains one-shot: repeated connect() calls are allowed while open, but reconnecting after failure or close is rejected.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16335

How was this patch tested?

Local Test

mvn -pl :ozone-integration-test -am test \
  -Dtest=TestXceiverClientManagerSC \
  -Dsurefire.failIfNoSpecifiedTests=false \
  -DskipShade -DskipRecon -DskipDocs

GitHub Actions CI: https://github.com/echonesis/ozone/actions/runs/34804142710

Generated-by: Codex (GPT-5)

[Gargi-jais11]

Thanks @echonesis for working on the PR. Left few review comemnts

[Gargi-jais11]

In sendRequest(), when the write faills we are currently fail all entries in pending, but metrics.decrPendingContainerOpsMetrics() and addContainerOpsLatency() are only called once for the current request, so other in-flight requests will leave infleted pending metrics.

[echonesis]

Good catch. A write failure makes the shared domain socket unusable, so all requests still registered in sentRequests need terminal metrics accounting, rather than only the request whose write failed.

I changed this path to conditionally remove and collect the pending entries while holding the client lock, then fail each collected request and update its pending and latency metrics after releasing the lock. The conditional removal also prevents duplicate accounting against a concurrent response or timeout.

[Gargi-jais11]

here when the close fails, we are not tracking any metrics failures.

[echonesis]

Thanks for mentioning.
close() now removes and collects pending entries under the client lock, then fails and accounts for each request outside the lock. Conditional removal ensures that repeated or concurrent close() calls cannot account for the same request more than once.

[Gargi-jais11]

In the generic catch (Throwable e) block,
if entry != null you are calling the entry.getFuture().completeExceptionally(e) and then pending.forEach(i -> i.fail(e)). If entry is still in sentRequests, it gets failed twice. CompletableFuture ignores the second completion, so this is harmless but redundant.

[echonesis]

The current receiver entry had already been removed from sentRequests, so it was not normally part of the snapshot taken later in the same catch block. However, snapshot-based failure handling could still overlap with another close or failure path.

I replaced the snapshots with explicit removal ownership. The receiver accounts for the entry it removed, while the common failure path handles only entries it can conditionally remove from sentRequests. This avoids duplicate future completion and metrics accounting across terminal paths.

[Gargi-jais11]

Nit: After failing pending requests on close, consider clearing sentRequests under the lock.
Similarly in sendRequests and recieveResponseTask

[echonesis]

The close, write failure, and receiver failure paths now remove pending requests under the client lock. Future completion and metrics updates happen after releasing the lock because completing a CompletableFuture may synchronously invoke callbacks, including close().

I used remove(key, entry) instead of snapshot followed by clear(), since timeout and response removal intentionally remain lock-free. Conditional removal gives each request a single terminal owner while ensuring failed entries are removed from the map.