Andrey Yarovoy created HDDS-15849:
-------------------------------------
Summary: XceiverClientGrpc.close() performs a blocking graceful
channel shutdown while holding the XceiverClientManager cache lock, serializing
concurrent client acquisition
Key: HDDS-15849
URL: https://issues.apache.org/jira/browse/HDDS-15849
Project: Apache Ozone
Issue Type: Bug
Components: Ozone Client
Reporter: Andrey Yarovoy
HDDS-14571 ("Remove synchronized methods from XceiverClientGrpc") changed
{{XceiverClientGrpc.close()}} from a forced shutdown to a graceful one:
* Before: {{channel.shutdownNow()}} followed by {{{}awaitTermination(...){}}}.
{{shutdownNow()}} cancels in-flight RPCs, so the channel reaches {{TERMINATED}}
almost immediately and {{close()}} returns in ~1 ms.
* After: {{channel.shutdown()}} (graceful drain) followed by a polling loop:
{{while (!nonTerminatedChannels.isEmpty() && System.nanoTime() < deadline) \{
nonTerminatedChannels.removeIf(ManagedChannel::isTerminated);
Thread.sleep(SHUTDOWN_WAIT_INTERVAL_MILLIS); // 100 ms
}}}
The loop always sleeps at least one {{SHUTDOWN_WAIT_INTERVAL_MILLIS}} (100 ms)
interval before it can observe termination, and waits up to
{{SHUTDOWN_WAIT_MAX_SECONDS}} (5 s). So every {{close()}} now costs *≥100 ms*
instead of ~1 ms.
*Why this is a problem beyond {{close()}} latency*
{{XceiverClientSpi}} clients are closed through cache eviction, and the
eviction machinery runs under the {{XceiverClientManager.clientCache}} monitor:
# {{acquireClient()}} holds {{synchronized (clientCache)}} and calls
{{{}clientCache.get(key, () -> newClient(...)){}}}.
# Inserting a new entry can evict an LRU/stale entry, and Guava runs the
removal listener {*}synchronously on the calling thread{*}.
# The removal listener takes {{synchronized (clientCache)}} (reentrant) →
{{setEvicted()}} → {{cleanup()}} → when refcount is 0, {{{}close(){}}}.
As a result, the ≥100 ms graceful-shutdown sleep now executes {*}while the
{{clientCache}} monitor is held{*}. All other threads calling
{{{}acquireClient(){}}}/{{{}releaseClient(){}}} block on that monitor for the
duration. With {{shutdownNow()}} the monitor was held for ~1 ms; with the
graceful path it is held for ≥100 ms per closed channel, converting the client
cache into a global serialization point under any workload that churns clients.
*Impact*
Workloads that create and evict many short-lived clients — in particular
parallel readers that open a client per pipeline/block and let the cache evict
them (e.g. EC read/checksum collection, where a distinct client is used per
placement group) — degrade sharply and non-linearly as concurrency increases,
because the per-close cost is paid serially under the cache lock. Wall-clock
profiling of such a workload shows threads parked in
{{{}XceiverClientManager.acquireClient{}}}/{{{}releaseClient{}}} and
{{{}XceiverClientGrpc.close{}}}, all contending on the {{clientCache}} monitor.
*Proposed fix (options)*
# Do not perform a blocking shutdown while holding the {{clientCache}}
monitor. The removal listener / {{cleanup()}} should not synchronously drive a
graceful {{close()}} under the lock — e.g. hand the channel shutdown to an
executor, or perform only the non-blocking {{channel.shutdown()}} synchronously
and drain/await outside the lock.
# Or restore {{shutdownNow()}} semantics for client eviction so termination is
effectively immediate (optionally retaining a graceful path only for explicit,
non-cache-driven closes).
# At minimum, avoid the mandatory {{Thread.sleep(100 ms)}} when all channels
are already terminated (check termination before sleeping), so an
already-drained channel closes without a fixed 100 ms penalty.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
