szetszwo commented on code in PR #10633:
URL: https://github.com/apache/ozone/pull/10633#discussion_r3623837258
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -244,11 +244,20 @@ protected List<OmKeyLocationInfo> allocateBlock(
for (AllocatedBlock allocatedBlock : allocatedBlocks) {
BlockID blockID = new BlockID(allocatedBlock.getBlockID());
Pipeline pipeline = allocatedBlock.getPipeline();
- if (sortedByPipeline != null) {
+ if (sortedByNodes != null) {
final List<DatanodeDetails> nodes = pipeline.getNodes();
- final List<? extends DatanodeDetails> sorted = sortedByPipeline
- .computeIfAbsent(pipeline.getId(),
- id -> keyManager.sortDatanodesForWrite(nodes,
omClientMachine));
+ final Set<String> uuidSet = nodes.stream()
+ .map(DatanodeDetails::getUuidString).collect(Collectors.toSet());
+ List<? extends DatanodeDetails> sorted = sortedByNodes.get(uuidSet);
+ if (sorted == null) {
+ sorted = keyManager.sortDatanodesForWrite(nodes, omClientMachine,
clusterMap);
+ // Cache only a freshly sorted order, not an input list returned
+ // unchanged when the client is unresolved: that order is
per-pipeline
+ // and must not be reused for another pipeline with the same node
set.
+ if (sorted != nodes) {
Review Comment:
Since the client is fixed, if the client cannot be resolved, it should break
the loop.
It is even better to
- resolve the client first before enterring the loop so the same client
won't be resolved again and again.
- When the client cannot be resolved, fallback to SCM.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
Review Comment:
It is better to return null, since checking `sortedNodes != nodes` similar
like a bug -- people may change it to use `equals(..)` later on.
--
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]
