ivandika3 commented on code in PR #10573:
URL: https://github.com/apache/ozone/pull/10573#discussion_r3510926676


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineReportHandler.java:
##########
@@ -139,6 +146,28 @@ protected void processPipelineReport(PipelineReport report,
     }
   }
 
+  private void updateSupportedStorageTiers(Pipeline pipeline) {
+    try {
+      Set<StorageTier> currentSupportedTiers =
+          pipeline.getSupportedStorageTier() != null
+              ? new HashSet<>(pipeline.getSupportedStorageTier())
+              : Collections.emptySet();
+
+      Set<StorageTier> newSupportedTiers =
+          new HashSet<>(NodeUtils.getDatanodesStorageTypes(pipeline.getNodes(),
+              scmContext.getScm().getScmNodeManager()));
+
+      if (!currentSupportedTiers.equals(newSupportedTiers)) {
+        pipeline.setSupportedStorageTier(new ArrayList<>(newSupportedTiers));
+        LOGGER.info("Updated supported storage tiers for Pipeline ID {} from 
{} to {}",
+            pipeline.getId(), currentSupportedTiers, newSupportedTiers);
+      }
+    } catch (Exception e) {
+      LOGGER.warn("Failed to update supported storage tiers for Pipeline ID {} 
due to: {}",
+          pipeline.getId(), e.getMessage(), e);
+    }
+  }

Review Comment:
   Please remove this



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java:
##########
@@ -188,13 +190,20 @@ public synchronized Pipeline 
create(RatisReplicationConfig replicationConfig,
 
     DatanodeDetails suggestedLeader = leaderChoosePolicy.chooseLeader(dns);
 
+    List<StorageTier> storageTiers = NodeUtils.getDatanodesStorageTypes(dns, 
getNodeManager());
+    if (!storageTiers.contains(storageTier)) {
+      throw new SCMException(String.format("Cannot create pipeline for 
StorageTier %s replicationConfig: %s",
+              storageTier, replicationConfig), 
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+    }

Review Comment:
   Don't set based on `NodeUtils.getdatanodeStorageTypes`, set a singleton tier.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##########
@@ -722,6 +724,49 @@ public void processNodeReport(DatanodeDetails 
datanodeDetails,
     }
   }
 
+  private void updateSupportedStorageTier(DatanodeInfo datanodeInfo) {

Review Comment:
   Remove this.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SimplePipelineProvider.java:
##########
@@ -73,33 +76,47 @@ public Pipeline create(StandaloneReplicationConfig 
replicationConfig,
     }
 
     Collections.shuffle(dns);
+    List<StorageTier> storageTiers = NodeUtils.getDatanodesStorageTypes(dns, 
getNodeManager());
+    if (!storageTiers.contains(storageTier)) {
+      throw new SCMException(String.format("Cannot create pipeline for 
StorageTier %s replicationConfig: %s",
+          storageTier, replicationConfig), 
SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE);
+    }

Review Comment:
   Same here, set singleton storage tier.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/ECPipelineProvider.java:
##########
@@ -135,17 +143,19 @@ public Pipeline createForRead(
 
     dns.sort(Comparator.comparing(nodeStatusMap::get, 
CREATE_FOR_READ_COMPARATOR));
 
-    return createPipelineInternal(replicationConfig, dns, map);
+    // Read Pipelines do not require storage tiers, so the calculation of 
storage tiers can be omitted.
+    return createPipelineInternal(replicationConfig, dns, map, new 
ArrayList<>());
   }
 
   private Pipeline createPipelineInternal(ECReplicationConfig repConfig,
-      List<DatanodeDetails> dns, Map<DatanodeDetails, Integer> indexes) {
+      List<DatanodeDetails> dns, Map<DatanodeDetails, Integer> indexes, 
List<StorageTier> storageTiers) {
     return Pipeline.newBuilder()
         .setId(PipelineID.randomId())
         .setState(Pipeline.PipelineState.ALLOCATED)
         .setReplicationConfig(repConfig)
         .setNodes(dns)
         .setReplicaIndexes(indexes)
+        .setSupportedStorageTier(storageTiers)

Review Comment:
   Make this singleton



-- 
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]

Reply via email to