chihsuan commented on code in PR #10633:
URL: https://github.com/apache/ozone/pull/10633#discussion_r3563550265
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -2165,7 +2166,7 @@ private void sortDatanodes(String clientMachine,
OmKeyInfo keyInfo) {
private void sortDatanodes(String clientMachine, List<OmKeyInfo> keyInfos) {
if (keyInfos != null && clientMachine != null) {
- final Map<Set<String>, List<? extends DatanodeDetails>> sortedPipelines
= new HashMap<>();
+ final Map<PipelineID, List<? extends DatanodeDetails>> sortedPipelines =
new HashMap<>();
Review Comment:
Good question! I switched it to PipelineID because Copilot
[flagged](https://github.com/apache/ozone/pull/10633#discussion_r3559080453)
the new write-path cache for using an order-sensitive List key and aligned the
read path to match for consistency.
> One difference I can see is that the set of UUIDs (i.e. datanodes) can
have multiple pipelines. In such case, we sort the same set of datanodes
multiple times. Not sure if this case is common.
That's a fair point, and it changes my mind. The sort depends only on the
datanode set, so keying by the set collapses them into a single sort, while
PipelineID re-sorts each. It's never worse than PipelineID, and it's what the
read path is already used on master. I've reverted both to the UUID set
(7307ae2015).
--
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]
