swamirishi commented on code in PR #8887:
URL: https://github.com/apache/ozone/pull/8887#discussion_r2266931053


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -261,6 +268,29 @@ public void verifyAndFixupContainerData(ContainerData 
containerData)
     }
   }
 
+  private boolean isMatchedLastLoadedECContainer(
+      KeyValueContainer kvContainer, WitnessedContainerMetadataStore 
containerMetadataStore) throws IOException {
+    if (null != containerMetadataStore && 
kvContainer.getContainerData().getReplicaIndex() != 0) {
+      ContainerCreateInfo containerCreateInfo = 
containerMetadataStore.getContainerCreateInfoTable()
+          
.get(ContainerID.valueOf(kvContainer.getContainerData().getContainerID()));
+      // check for EC container replica index matching if db entry is present 
for container as last loaded,
+      // and delete if not matched, ignoring replica index -1 as no previous 
replica index information
+      if (null != containerCreateInfo && containerCreateInfo.getReplicaIndex() 
>= 0
+          && containerCreateInfo.getReplicaIndex() != 
kvContainer.getContainerData().getReplicaIndex()) {
+        LOG.info("EC Container {} with replica index {} present at path {} is 
not matched with DB replica index {}," +
+                " deleting the container data.",
+            kvContainer.getContainerData().getContainerID(),
+            kvContainer.getContainerData().getReplicaIndex(),
+            kvContainer.getContainerData().getContainerPath(),
+            containerCreateInfo.getReplicaIndex());
+        KeyValueContainerUtil.removeContainer(kvContainer.getContainerData(), 
hddsVolume.getConf());

Review Comment:
   Let us not remove this upfront. We can remove this replica if another 
replica with the correct replica index exists. So if the container is already 
loaded in the memory with incorrect replica index and the correct one gets 
loaded let us delete the replica then.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/WitnessedContainerMetadataStoreImpl.java:
##########
@@ -114,7 +114,7 @@ public void init(DBStore dbStore) throws 
RocksDatabaseException, CodecException
       if 
(!VersionedDatanodeFeatures.isFinalized(HDDSLayoutFeature.WITNESSED_CONTAINER_DB_PROTO_VALUE))
 {
         this.containerIdsTable = dbStore.getTable(CONTAINER_IDS_STR_VAL_TABLE, 
ContainerID.getCodec(),
             new DelegatedCodec<>(StringCodec.get(),
-                (strVal) -> 
ContainerCreateInfo.valueOf(ContainerProtos.ContainerDataProto.State.valueOf(strVal)),
+                (strVal) -> 
ContainerCreateInfo.valueOf(ContainerProtos.ContainerDataProto.State.valueOf(strVal),
 -1),

Review Comment:
   Can we move the -1 to a constant field.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -261,6 +268,29 @@ public void verifyAndFixupContainerData(ContainerData 
containerData)
     }
   }
 
+  private boolean isMatchedLastLoadedECContainer(
+      KeyValueContainer kvContainer, WitnessedContainerMetadataStore 
containerMetadataStore) throws IOException {
+    if (null != containerMetadataStore && 
kvContainer.getContainerData().getReplicaIndex() != 0) {
+      ContainerCreateInfo containerCreateInfo = 
containerMetadataStore.getContainerCreateInfoTable()
+          
.get(ContainerID.valueOf(kvContainer.getContainerData().getContainerID()));
+      // check for EC container replica index matching if db entry is present 
for container as last loaded,
+      // and delete if not matched, ignoring replica index -1 as no previous 
replica index information
+      if (null != containerCreateInfo && containerCreateInfo.getReplicaIndex() 
>= 0
+          && containerCreateInfo.getReplicaIndex() != 
kvContainer.getContainerData().getReplicaIndex()) {
+        LOG.info("EC Container {} with replica index {} present at path {} is 
not matched with DB replica index {}," +
+                " deleting the container data.",
+            kvContainer.getContainerData().getContainerID(),
+            kvContainer.getContainerData().getReplicaIndex(),
+            kvContainer.getContainerData().getContainerPath(),
+            containerCreateInfo.getReplicaIndex());
+        KeyValueContainerUtil.removeContainer(kvContainer.getContainerData(), 
hddsVolume.getConf());

Review Comment:
   We should not remove replica index erroneosly since that could lead to error 
in reconstruction. I believe this can happen during upgrade.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/WitnessedContainerMetadataStoreImpl.java:
##########
@@ -114,7 +114,7 @@ public void init(DBStore dbStore) throws 
RocksDatabaseException, CodecException
       if 
(!VersionedDatanodeFeatures.isFinalized(HDDSLayoutFeature.WITNESSED_CONTAINER_DB_PROTO_VALUE))
 {
         this.containerIdsTable = dbStore.getTable(CONTAINER_IDS_STR_VAL_TABLE, 
ContainerID.getCodec(),
             new DelegatedCodec<>(StringCodec.get(),
-                (strVal) -> 
ContainerCreateInfo.valueOf(ContainerProtos.ContainerDataProto.State.valueOf(strVal)),
+                (strVal) -> 
ContainerCreateInfo.valueOf(ContainerProtos.ContainerDataProto.State.valueOf(strVal),
 -1),

Review Comment:
   Shouldn't this be -1 for EC and 0 for ratis containers?



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