[GitHub] [ozone] sodonnel commented on a change in pull request #2963: HDDS-5267 Merge ICR and FCR for containers if part of the same HB

[GitBox <gi...@apache.org>]

sodonnel commented on a change in pull request #2963:
URL: https://github.com/apache/ozone/pull/2963#discussion_r786754897



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerSet.java
##########
@@ -235,18 +235,39 @@ public void listContainer(long startContainerId, long count,
   public ContainerReportsProto getContainerReport() throws IOException {
     LOG.debug("Starting container report iteration.");
 
+    ContainerReportsProto.Builder crBuilder =
+        ContainerReportsProto.newBuilder();
     // No need for locking since containerMap is a ConcurrentSkipListMap
     // And we can never get the exact state since close might happen
     // after we iterate a point.
     List<Container<?>> containers = new ArrayList<>(containerMap.values());
+    // Incremental Container reports can read stale container information
+    // This is to make sure FCR and ICR can be linearized and processed by
+    // consumers such as SCM.
+    synchronized (this) {
+      for (Container<?> container : containers) {
+        crBuilder.addReports(container.getContainerReport());
+      }
+    }
+    return crBuilder.build();
+  }
 
+  /**
+   * Get container report without lock.
+   *
+   * @return The container report.
+   */
+  public ContainerReportsProto getContainerReportUnlocked() throws IOException {

Review comment:
       I don't think we need this Unlocked method, and someone may mistakenly use it for something in the futher if we leave it. It seems to be used only in StateContext.java where we already synchronize on the ContainerSet, but its fine to call another method which synchronizes on the same object, as the synchronization is reentrant.




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org