xichen01 commented on code in PR #10810:
URL: https://github.com/apache/ozone/pull/10810#discussion_r3667740403


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java:
##########
@@ -458,6 +461,8 @@ private StorageContainerManager(OzoneConfiguration conf,
     scmAdmins = OzoneAdmins.getOzoneAdmins(scmStarterUser, conf);
     scmReadOnlyAdmins = OzoneAdmins.getReadonlyAdmins(conf);
     LOG.info("SCM start with adminUsers: {}", scmAdmins.getAdminUsernames());
+    StorageTier.setDefault(StorageTier.valueOf(conf.get(

Review Comment:
   We can handle both uppercase and lowercase letters.



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerInfo.java:
##########
@@ -290,6 +298,16 @@ public void setSuppressed(boolean suppressed) {
     this.suppressed = suppressed;
   }
 
+  @Nullable
+  @JsonIgnore
+  public StorageTier getStorageTier() {
+    return storageTier;
+  }
+
+  public void setStorageTier(StorageTier storageTier) {

Review Comment:
   This can be removed, it's not needed for the current commit.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java:
##########
@@ -523,14 +527,17 @@ public ContainerInfo getMatchingContainer(final long 
size, String owner,
     return selectedContainer;
   }
 
-  private ContainerInfo findContainerWithSpace(final long size,
-                                               final NavigableSet<ContainerID>
-                                                   searchSet) {
-      // Get the container with space to meet our request.
+  private ContainerInfo findContainerWithSpaceAndStorageTier(final long size,
+      final NavigableSet<ContainerID> searchSet, @Nonnull StorageTier 
storageTier) {
+      // Get the container with space to meet our request. Containers with a
+      // null storageTier are treated as matching any tier (upgrade-compat with
+      // pre-storageTier containers).
     for (ContainerID id : searchSet) {
       try (AutoCloseableLock ignored = readLock(id)) {
         final ContainerInfo containerInfo = containers.getContainerInfo(id);
-        if (containerInfo.getUsedBytes() + size <= this.containerSize) {
+        if (containerInfo.getUsedBytes() + size <= this.containerSize &&
+            (containerInfo.getStorageTier() == null ||

Review Comment:
   It is best to use exact matching here (`containerInfo.getStorageTier() != 
null`); 
   
   later, if we want to ensure a smooth upgrade process, we can add a conf to 
allow containers with a null `StorageTier` to be selected.
   



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerInfo.java:
##########
@@ -290,6 +298,16 @@ public void setSuppressed(boolean suppressed) {
     this.suppressed = suppressed;
   }
 
+  @Nullable
+  @JsonIgnore

Review Comment:
   we can display this `storageTier`
   



##########
hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/UniformDatanodesFactory.java:
##########
@@ -184,6 +208,18 @@ public Builder setCurrentVersion(DatanodeVersion version) {
       return this;
     }
 
+    /**
+     * Per-datanode storage type list. Outer list is indexed by datanode; 
inner list
+     * is indexed by volume within a datanode. Each inner list, when 
non-empty, must
+     * have size == numDataVolumes. When set, each data dir is prefixed with

Review Comment:
   can add a check for this



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