Copilot commented on code in PR #10633:
URL: https://github.com/apache/ozone/pull/10633#discussion_r3563539016
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -215,11 +242,23 @@ protected List<OmKeyLocationInfo> allocateBlock(
}
for (AllocatedBlock allocatedBlock : allocatedBlocks) {
BlockID blockID = new BlockID(allocatedBlock.getBlockID());
+ Pipeline pipeline = allocatedBlock.getPipeline();
+ if (sortedByNodes != null) {
+ final List<DatanodeDetails> nodes = pipeline.getNodes();
+ final Set<String> uuidSet = nodes.stream()
+ .map(DatanodeDetails::getUuidString).collect(Collectors.toSet());
+ final List<? extends DatanodeDetails> sorted = sortedByNodes
+ .computeIfAbsent(uuidSet,
+ k -> keyManager.sortDatanodesForWrite(nodes, omClientMachine,
clusterMap));
+ if (!Objects.equals(sorted, pipeline.getNodesInOrder())) {
+ pipeline = pipeline.copyWithNodesInOrder(sorted);
+ }
+ }
Review Comment:
`sortedByNodes` caches by an order-insensitive UUID set, but
`KeyManager#sortDatanodesForWrite` returns the *input list instance* when no
sort should happen (eg client unresolved / stale topology). Caching that
unsorted result can then be reused for a different pipeline with the same node
set but a different original order, which breaks the contract of preserving the
original pipeline order for writes when the client cannot be resolved. Avoid
caching when the returned list is the same instance as the input (ie no sort
actually performed).
--
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]
