greenwich opened a new pull request, #11840: URL: https://github.com/apache/ozone/pull/11840
## What changes were proposed in this pull request? Since HDDS-9343 (2.0.0), OM sorts each block's datanodes for reads itself: `KeyManagerImpl.sortDatanodes` → `NetworkTopologyImpl.sortByDistanceCost`. Those nodes come from `pipeline.getNodes()`. They reach OM from SCM over RPC, so they are built by `DatanodeDetails.getFromProtoBuf` and carry `networkLocation` and `level` but no parent. `getDistanceCost` walks parent pointers, finds none, and returns `Integer.MAX_VALUE` for every node except the client itself. Reads end up "same host first, then random", and `ozone.network.topology.aware.read=true` (the default) gives no rack locality. Before HDDS-9343, SCM sorted on its own linked nodes and rack order worked. HDDS-15059 fixed the same problem for streaming writes, but not for reads. Reproduced with 6 datanodes on 3 racks and a client on `/r1`, using proto round-tripped nodes as OM receives them. Both same-rack nodes were sorted first in **76/1000** sorts, which is random chance (≈6.7%), against **1000/1000** with linked nodes. ### Changes `NetworkTopologyImpl.sortByDistanceCost`: when the reader or a node is not linked into the tree, its cost comes from the two network locations and the schema's per-level costs (`getDistanceCostByLocation`). HDFS does the same in `NetworkTopology#getWeightUsingNetworkLocation`. - Inner nodes take their level's schema cost, so for linked nodes this gives exactly what `getDistanceCost` gives. A same-rack node costs 2 and an other-rack node costs 4 on the default schema, and custom per-level costs are respected. - Same host is still detected by `equals` (UUID for datanodes) and costs 0. - No tree lookup and no lock. On OM's read path every pipeline node takes this branch, so the read sort no longer takes `netlock` once per node. - `getDistanceCost` is unchanged. SCM placement and other callers still get `MAX_VALUE` for nodes outside the tree. ### Edge cases - **Location depth doesn't match the schema** (for example `/r1/extra` on a rack-only schema), or the location doesn't parse: cost `MAX_VALUE`, so the node sorts last, as today. - **Datanode not yet in OM's map:** a datanode SCM knows but OM's map (refreshed hourly by default) doesn't have yet now gets its rack cost from its location, instead of `MAX_VALUE`. - **Schema:** the costs come from OM's schema. OM and SCM must use the same schema, which is already required for OM's cluster map. - **Client not resolved:** the client is `null` as before, and the order is random, as before. - **Behaviour change:** RATIS reads now prefer same-rack datanodes after same host, instead of a random datanode. Read load can shift towards datanodes that share racks with clients. EC reads are not affected, because `ECBlockInputStream` picks nodes by replica index. - **SCM:** SCM's `sortDatanodes` resolves nodes through its node manager, so they are linked and take the existing path. Unlinked pipeline nodes sorted in `allocateBlock` now get location costs too. - **HDDS-17553:** it also touches `sortByDistanceCost` (a lock-free walk for linked nodes). The two are independent; whichever lands second needs a small rebase. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-17555 ## How was this patch tested? New tests: - `TestNetworkTopologyImpl`: - `testSortByDistanceCostOfUnlinkedNodesMatchesLinked`, on all schema topologies: sorting unlinked copies gives the same order as sorting linked nodes, for each combination of linked or unlinked reader and nodes. - `testSortByDistanceCostOfUnlinkedNodesWithCustomCosts`: the same check with non-default per-level costs. - `testSortByDistanceCostOrdersUnlinkedNodesByRack`: a proto-style pipeline gets same-rack nodes first for a non-datanode client, and the datanode itself, then its rack peer, for a datanode reader. - `testSortByDistanceCostPutsUnlinkedNodeWithUnknownDepthLast`: a node whose location has the wrong depth sorts last, and `getDistanceCost` still returns `MAX_VALUE` for it. - `TestOMSortDatanodes.sortDatanodesSortsRpcDeserializedNodesByRack`: end to end through `KeyManagerImpl.sortDatanodes` with proto round-tripped nodes. Datanode clients get themselves first, then their rack; edge clients get their rack first. All of the new tests fail without the fix. Existing tests passing: - `TestNetworkTopologyImpl` (including `testCost` and `testSortByDistanceCost`), `TestOMSortDatanodes`, and both `TestKeyManagerImpl` classes. - `TestSCMBlockProtocolServer`, `TestPipelinePlacementPolicy`, and `TestSCMContainerPlacement*` with `TestContainerPlacementFactory`. -- 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]
