greenwich opened a new pull request, #11838:
URL: https://github.com/apache/ozone/pull/11838
## What changes were proposed in this pull request?
On every read that returns block locations (`GetKeyInfo`, `lookupKey`,
`getFileStatus`, `listStatus`), OM sorts each block's datanodes by distance to
the client: `KeyManagerImpl.sortDatanodes` →
`NetworkTopologyImpl.sortByDistanceCost` → `getDistanceCost` **once per node**.
Each `getDistanceCost` call takes the read lock of `netlock`, a single global
fair `ReentrantReadWriteLock`. For non-datanode clients, `getOtherNode` also
takes it once per request to look up the client's rack.
With many concurrent readers, every read-lock acquisition CASes one shared
state word and looks up a per-thread hold count in a `ThreadLocal`. With 100+
handlers this turns into CAS retries and long `ThreadLocal` probes, OM CPU
climbs while throughput falls, and slower calls keep more handlers in flight.
Observed in production (2.2.1, 3-node OM HA, ~32 datanodes, 168-core OM
host):
- The OM leader bursts from a 3–5 core baseline to **80–135 cores for 1–5
minutes**, 20+ times a day, as concurrent S3 GETs rise to ~15–30k/s.
- During a burst, `GetKeyInfo` takes **~6 ms** 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, ~70% of the call.
- JFR on the IPC handlers: **68%** of CPU samples in
`ThreadLocal$ThreadLocalMap.getEntryAfterMiss` (9% at baseline). Of those,
**85%** come from `ReadLock.lock` in `getDistanceCost` ← `sortByDistanceCost`,
and **15%** from `getNode` ← `KeyManagerImpl.getOtherNode`.
HDFS solved this long ago: `NetworkTopology` has the same fair `netlock`,
but `sortByDistance` computes weights with `getWeight`, a lock-free walk of
parent pointers that compares nodes by path (`NodeBase.equals`). This PR
applies the same approach.
### Changes
1. **`NetworkTopologyImpl`: lock-free distance walk for sorting.**
- The cost walk moves into `getDistanceCostWithoutLock`, which
`sortByDistanceCost` uses. `getDistanceCost` keeps the lock for its other
callers and delegates to the same method, so the two cannot diverge.
- Tree nodes are compared by full path, as HDFS does, instead of
`InnerNodeImpl.equals`. That method compares whole child maps when the two
nodes are different instances; without the lock it could iterate a map that SCM
is modifying.
- The cost formula is unchanged (schema `getCost()` sums,
`Integer.MAX_VALUE` for nodes outside the tree). HDFS's hop count is not used,
because Ozone schemas support per-level costs.
2. **`KeyManagerImpl`: cache resolved non-datanode clients.**
- `getOtherNode` caches the client `Node` per client host. This removes
the per-request `getNode` lock, the mapping lookup and a `NodeImpl` allocation.
- The cache belongs to one cluster-map instance. `ScmTopologyClient`
replaces the instance on refresh, and the cache is replaced with it, so a
cached node always points at the current tree.
- It holds at most 10,000 entries, and entries expire after
`ozone.om.network.topology.refresh.duration` (default 1h).
### Edge cases
- **OM:** the cluster map is built once, published through an
`AtomicReference`, and never modified, so the lock protected nothing there.
Results are identical.
- **SCM:** the topology does change, and a concurrent change can make one
sort see a stale cost. That's the same tolerance HDFS accepts, and it's
harmless for replica ordering. If a node is removed mid-walk, so that its
parent becomes `null` after the ancestor check, the walk returns `MAX_VALUE`
instead of a partial cost, so a node being removed never sorts first.
- **Path comparison changes two cases, neither expected in practice:**
- A node attached to another topology instance with the same paths now
gets its real cost instead of being treated as outside the cluster.
- A non-datanode client whose hostname equals a datanode's network name on
the same rack costs 0 (same host) instead of 2. The read path matches such
clients to the datanode before this point.
- **Clients on a rack with no datanodes:** the rack is not in OM's map. This
result is cached too, because rack existence only changes with the map instance.
- **Resolution failures:** when the mapping returns nothing or throws, the
result is not cached and is retried on the next request, as today (HDDS-16776
is unchanged).
- **Mappings that don't cache:** the default `ScriptBasedMapping` caches
successful lookups forever, so nothing changes for it. A mapping configured
directly, such as `StaticMapping` or a custom class, is now consulted at most
once per refresh interval per client.
- **Unchanged:**
- When the client is itself a pipeline datanode, OM still does one
`getNode` lookup per request.
- Read-path rack locality is unchanged. Pipeline nodes deserialized from
SCM are not linked into OM's map, so they still score `MAX_VALUE` (cheaply
now). That is fixed separately in HDDS-17555.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-17553
## How was this patch tested?
New tests:
- `TestNetworkTopologyImpl`:
- `testSortByDistanceCostDoesNotTakeLockPerNode`: sorting never calls the
locked `getDistanceCost`. It fails on the old code.
- `testSortByDistanceCostWhileTopologyChanges`: four threads sort while a
fifth removes and re-adds half the nodes, on all five schema topologies. Every
result contains all nodes, and the nodes that aren't changing stay ordered by
cost.
- `testDistanceCostIsMaxWhenNodeIsRemovedDuringWalk`: a node that loses
its parent after the ancestor check costs `MAX_VALUE`. The old code gave it 1,
below a same-rack node's 2.
- `TestOMSortDatanodes`:
- `cachesResolvedClientPerClusterMap`: same node for the same map, a new
one for a new map instance.
- `cachesClientOnRackWithoutDatanodes`: the "no rack in this map" result
is cached per map instance.
- `cachedClientExpiresAfterTopologyRefreshInterval`: an entry expires
after the refresh interval.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
