JacksonYao287 commented on a change in pull request #2182:
URL: https://github.com/apache/ozone/pull/2182#discussion_r644554992



##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java
##########
@@ -332,19 +363,21 @@ public void putBackReports(List<GeneratedMessage> 
reportsToPutBack,
     return reportsToReturn;
   }
 
-  List<GeneratedMessage> getNonIncrementalReports() {
+  List<GeneratedMessage> getNonIncrementalReports(

Review comment:
       sure, of course!

##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java
##########
@@ -116,6 +113,11 @@
   private boolean shutdownOnError = false;
   private boolean shutdownGracefully = false;
   private final AtomicLong threadPoolNotAvailableCount;
+  private Map<InetSocketAddress,
+      Map<String, AtomicBoolean>> fullReportSendIndicator;
+
+  private List<String> fullReportTypeList;
+  private Map<String, AtomicReference<GeneratedMessage>> type2Reports;

Review comment:
       Thanks @smengcl very much for  the review!  your notes are exactly 
correct. will commit this

##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java
##########
@@ -332,19 +363,21 @@ public void putBackReports(List<GeneratedMessage> 
reportsToPutBack,
     return reportsToReturn;
   }
 
-  List<GeneratedMessage> getNonIncrementalReports() {
+  List<GeneratedMessage> getNonIncrementalReports(
+      InetSocketAddress endpoint) {
+    Map<String, AtomicBoolean> mp = fullReportSendIndicator.get(endpoint);
     List<GeneratedMessage> nonIncrementalReports = new LinkedList<>();
-    GeneratedMessage report = containerReports.get();
-    if (report != null) {
-      nonIncrementalReports.add(report);
-    }
-    report = nodeReport.get();
-    if (report != null) {
-      nonIncrementalReports.add(report);
-    }
-    report = pipelineReports.get();
-    if (report != null) {
-      nonIncrementalReports.add(report);
+    if (null != mp){
+      for (Map.Entry<String, AtomicBoolean> kv : mp.entrySet()) {
+        if (kv.getValue().get()) {
+          String reportType = kv.getKey();
+          GeneratedMessage msg = type2Reports.get(reportType).get();
+          if (null != msg) {
+            nonIncrementalReports.add(msg);
+            mp.get(reportType).set(false);
+          }

Review comment:
       yes, this does make sense. i will update this with a new commit , in 
which i will also resolve the conflict with mater branch

##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java
##########
@@ -332,19 +363,21 @@ public void putBackReports(List<GeneratedMessage> 
reportsToPutBack,
     return reportsToReturn;
   }
 
-  List<GeneratedMessage> getNonIncrementalReports() {
+  List<GeneratedMessage> getNonIncrementalReports(
+      InetSocketAddress endpoint) {
+    Map<String, AtomicBoolean> mp = fullReportSendIndicator.get(endpoint);
     List<GeneratedMessage> nonIncrementalReports = new LinkedList<>();
-    GeneratedMessage report = containerReports.get();
-    if (report != null) {
-      nonIncrementalReports.add(report);
-    }
-    report = nodeReport.get();
-    if (report != null) {
-      nonIncrementalReports.add(report);
-    }
-    report = pipelineReports.get();
-    if (report != null) {
-      nonIncrementalReports.add(report);
+    if (null != mp){
+      for (Map.Entry<String, AtomicBoolean> kv : mp.entrySet()) {
+        if (kv.getValue().get()) {
+          String reportType = kv.getKey();
+          GeneratedMessage msg = type2Reports.get(reportType).get();
+          if (null != msg) {
+            nonIncrementalReports.add(msg);
+            mp.get(reportType).set(false);
+          }

Review comment:
       for now, maybe we can not do this change , because it will make several 
unit test fail.
   see `TestDatanodeStateMachine#testDatanodeStateContext`, 
`TestStateContext#testReportAPIs`, and 
`TestHeartbeatEndpointTask#testheartbeatWithNodeReport`. in some test case , 
some of nonIncrementalReportType is not set, so `msg == null ` will be a normal 
behavior and the CI will fail with throwing an exception .  may be we can fix 
these case in another jira later.




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

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