smengcl commented on a change in pull request #2874:
URL: https://github.com/apache/ozone/pull/2874#discussion_r762208410



##########
File path: 
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ContainerReplicaHistory.java
##########
@@ -37,11 +39,22 @@
   // Last reported time of the replica
   private Long lastSeenTime;
 
+  public void setBcsId(long bcsId) {
+    this.bcsId = bcsId;
+  }
+

Review comment:
       nit: ordering. move the setter next to its corresponding getter below.

##########
File path: 
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerHealthSchemaManager.java
##########
@@ -69,8 +71,14 @@ public ContainerHealthSchemaManager(
     SelectQuery<Record> query = dslContext.selectQuery();
     query.addFrom(UNHEALTHY_CONTAINERS);
     if (state != null) {
-      query.addConditions(
-          UNHEALTHY_CONTAINERS.CONTAINER_STATE.eq(state.toString()));
+      if (state.equals(ALL_REPLICAS_UNHEALTHY)) {
+        query.addConditions(UNHEALTHY_CONTAINERS.CONTAINER_STATE
+            .eq(UNDER_REPLICATED.toString()));
+        query.addConditions(UNHEALTHY_CONTAINERS.ACTUAL_REPLICA_COUNT.eq(0));

Review comment:
       Q: In which other cases are `ACTUAL_REPLICA_COUNT` set? Apart from this 
`ALL_REPLICAS_UNHEALTHY` case.

##########
File path: 
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconDBDefinition.java
##########
@@ -74,6 +74,16 @@ public ReconDBDefinition(String dbName) {
           NSSummary.class,
           new NSSummaryCodec());
 
+  // Container Replica History with bcsId tracking.
+  public static final DBColumnFamilyDefinition
+      <Long, ContainerReplicaHistoryList> REPLICA_HISTORY_V2 =
+      new DBColumnFamilyDefinition<Long, ContainerReplicaHistoryList>(
+          "replica_history_v2",

Review comment:
       So in a future Recon upgrade process we should probably drop (or the 
upgrade layout of) `REPLICA_HISTORY` (V1) table right? to avoid wasted storage 
space.

##########
File path: 
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/persistence/ContainerHistory.java
##########
@@ -30,14 +30,21 @@
   private String datanodeHost;
   private long firstSeenTime;
   private long lastSeenTime;
+  private long bcsId;

Review comment:
       Note to myself: `bcsId` stands for `blockCommitSequenceId`




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