[ 
https://issues.apache.org/jira/browse/HDDS-17553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18133992#comment-18133992
 ] 

Aleksei Ieshin commented on HDDS-17553:
---------------------------------------

Other related jiras
 * HDDS-15805: {{sortByDistanceCost}} should account for racks (same method).
 * HDDS-16776: a failing topology script is re-run on every request (the same 
client-resolution path).
 * HDDS-10357: granular metrics for OM {{{}sortDatanodes{}}}.
 * HDDS-15059: optional OM-side sort for streaming writes 
({{{}ozone.om.block.write.sort.datanodes.enabled{}}}, default off). In its 
review ([PR #10633|https://github.com/apache/ozone/pull/10633]) the OM CPU cost 
of sorting on OM was raised ("OM is supposed to be more CPU demanding than SCM. 
So, we probably want to save the CPU cycles in OM") but not measured. The 
read-path numbers above are such a measurement. When enabled, it adds more 
{{{}sortByDistanceCost{}}}/{{{}getNode{}}} calls on OM.
 * HDDS-15941 (Patch Available): resolves the client once per request on the 
write path. It's the same idea as part 2 here, for 
{{{}sortDatanodesForWrite{}}}, and touches the same code.
 * HDDS-9343 / HDDS-9272: moved read-path sorting from SCM into OM to remove 
the SCM round trip (HDDS-9272 measured it at >80% of {{getKeyInfo}} latency). 
That move is what put {{{}NetworkTopologyImpl{}}}'s per-node lock on OM's read 
path.

h2.  

> OM read path saturates CPU on NetworkTopologyImpl's global read lock in 
> sortDatanodes
> -------------------------------------------------------------------------------------
>
>                 Key: HDDS-17553
>                 URL: https://issues.apache.org/jira/browse/HDDS-17553
>             Project: Apache Ozone
>          Issue Type: Bug
>          Components: OM, SCM
>    Affects Versions: 2.2.1
>            Reporter: Aleksei Ieshin
>            Priority: Major
>
> h2. Problem
> On every read that returns block locations ({{{}GetKeyInfo{}}}, 
> {{{}lookupKey{}}}, {{{}getFileStatus{}}}, {{listStatus}} with locations), OM 
> sorts each block's datanodes by distance to the client:
> {{KeyManagerImpl.sortDatanodes}} → {{NetworkTopologyImpl.sortByDistanceCost}} 
> → {{getDistanceCost(reader, node)}} *once per node*
> {{getDistanceCost}} takes the read lock of 
> {{{}NetworkTopologyImpl.netlock{}}}, a single global {{new 
> ReentrantReadWriteLock(true)}} ({{{}NetworkTopologyImpl.java:65{}}}; the call 
> is at {{:799}} in {{{}sortByDistanceCost{}}}).
> Under read-heavy load this means one lock round-trip per node per block per 
> request, on a single fair lock shared by every RPC handler. Reads still have 
> to CAS one shared state word and look up a per-thread hold count in a 
> ThreadLocal. With 100+ handlers on a large host, that turns into CAS retry 
> loops and long ThreadLocal probes, and OM CPU climbs while throughput falls. 
> It feeds on itself: slower calls mean more handlers in flight and more 
> contention.
> HDFS takes no such lock on this path. 
> {{org.apache.hadoop.net.NetworkTopology}} uses the same fair {{{}netlock{}}}, 
> but {{sortByDistance}} computes weights with {{getWeight}} / 
> {{{}getWeightUsingNetworkLocation{}}}, a *lock-free* walk of parent pointers 
> or path strings. The lock is taken only for topology changes.
> h2. Observed (production, 2.2.1, 3-node OM HA, ~32 DNs, OM host with 168 
> cores)
>  * The OM leader repeatedly bursts from a 3–5 core baseline to {*}80–135 
> cores for 1–5 minutes{*}, 20+ times a day. The trigger is a rise in 
> concurrent S3 GETs (~15–30k/s), not a change in request mix.
>  * During a burst, {{GetKeyInfo}} takes *~6 ms per call* against ~1.5–2.5 ms 
> at baseline, at a _lower_ call rate.
>  * 
> {{{}om_performance_metrics_get_key_info_sort_datanodes_latency_ns_avg_time{}}}:
>  *0.01 ms* quiet, *4.2 ms* at the burst peak. The sort step alone is ~70% of 
> the call.
>  * JFR ({{{}profile{}}} settings), IPC handler threads, burst against the 
> preceding 10 minutes:
> ||Samples||Baseline||Burst||
> |CPU samples in {{ThreadLocal$ThreadLocalMap.getEntryAfterMiss}}|9%|*68%*|
> |... of which under {{ReentrantReadWriteLock.ReadLock.lock}} from 
> {{NetworkTopologyImpl.getDistanceCost}} ← {{sortByDistanceCost}} ← 
> {{KeyManagerImpl.sortDatanodes}}| |*85%*|
> |... from {{NetworkTopologyImpl.getNode}} ← {{KeyManagerImpl.getOtherNode}}| 
> |*15%*|
> Top of a burst stack:
> {noformat}
> java.lang.ThreadLocal$ThreadLocalMap.getEntryAfterMiss
> java.util.concurrent.locks.ReentrantReadWriteLock$Sync.fullTryAcquireShared
> java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryAcquireShared
> java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireShared
> java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.lock
> org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl.getDistanceCost
> org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl.sortByDistanceCost
> org.apache.hadoop.ozone.om.KeyManagerImpl.sortDatanodes
> ...
> org.apache.hadoop.ozone.om.KeyManagerImpl.getKeyInfo
> {noformat}
> The only workaround is {{ozone.network.topology.aware.read=false}} on 
> clients, which gives up topology-aware reads.
> h2. Proposed fix
>  # In {{{}NetworkTopologyImpl.sortByDistanceCost{}}}, compute costs with a 
> lock-free walk of the parent pointers, as HDFS {{NetworkTopology.getWeight}} 
> does. {{getDistanceCost}} stays as it is for its other callers. 
> Alternatively, take {{netlock}} once for the whole loop instead of once per 
> node; that's simpler, but still one acquisition per block.
>  # In {{{}KeyManagerImpl{}}}, avoid a {{getNode}} lock acquisition and 
> topology-script resolution per request for non-datanode clients 
> ({{{}getOtherNode{}}}), e.g. a small cache of resolved client nodes, 
> invalidated on topology refresh. This also overlaps with HDDS-16776.
> Both changes keep the sort result *identical* (same cost function, same 
> per-cost-group shuffle; nodes without a parent still score 
> {{{}Integer.MAX_VALUE{}}}). This is a pure performance change. The read-path 
> rack-ordering bug below is tracked separately; the two fixes are independent.
> h2. Related correctness issue (will be filed separately)
> Pipeline {{DatanodeDetails}} on the read path are deserialized from SCM's 
> protobuf. They carry {{level}} but no {{{}parent{}}}, and the read path 
> doesn't map them onto OM's cluster map. A unit test on 2.2.1 confirms that 
> {{getDistanceCost}} then returns {{Integer.MAX_VALUE}} for every node except 
> the client, _after_ taking the lock. Read ordering is therefore "same host 
> first, then random". A non-datanode client on rack r1 got its same-rack nodes 
> first in 76/1000 sorts (random), against 1000/1000 with linked nodes. So the 
> lock contention reported here buys no rack locality today.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to