[
https://issues.apache.org/jira/browse/HDDS-15849?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated HDDS-15849:
----------------------------------
Labels: pull-request-available (was: )
> 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
> Assignee: Andrey Yarovoy
> Priority: Major
> Labels: pull-request-available
>
> 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]
