You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "dombizita (via GitHub)" <gi...@apache.org> on 2023/02/27 14:50:08 UTC

[GitHub] [ozone] dombizita commented on a diff in pull request #4182: HDDS-5463. [FSO] Recon Container API does not work correctly with FSO.

dombizita commented on code in PR #4182:
URL: https://github.com/apache/ozone/pull/4182#discussion_r1118849207


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -90,18 +91,24 @@ public Pair<String, Boolean> reprocess(OMMetadataManager omMetadataManager) {
       reconContainerMetadataManager
               .reinitWithNewContainerDataFromOm(new HashMap<>());
 
-      Table<String, OmKeyInfo> omKeyInfoTable =
-          omMetadataManager.getKeyTable(getBucketLayout());
-      try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
-               keyIter = omKeyInfoTable.iterator()) {
-        while (keyIter.hasNext()) {
-          Table.KeyValue<String, OmKeyInfo> kv = keyIter.next();
-          OmKeyInfo omKeyInfo = kv.getValue();
-          handlePutOMKeyEvent(kv.getKey(), omKeyInfo, containerKeyMap,
-              containerKeyCountMap, deletedKeyCountList);
-          omKeyCount++;
+      // loop over both key table and file table
+      for (BucketLayout layout : Arrays.asList(BucketLayout.DEFAULT,

Review Comment:
   why did you choose to use `BucketLayout.DEFAULT` here instead of `BucketLayout.LEGACY` (like in `ContainerEndpoint` on [line 177](https://github.com/apache/ozone/blob/f6f42207ba0a6bf18a58145d352c4904250a32ef/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java#L177))? later the default bucket layout could change (I believe). I think we should use `BucketLayout.LEGACY` consistently (I am fine with `BucketLayout.DEFAULT` too, but it should be consistent) 



##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java:
##########
@@ -229,13 +239,63 @@ public void setUp() throws Exception {
     OMMetadataManager omMetadataManagerMock = mock(OMMetadataManager.class);
     Table tableMock = mock(Table.class);
     when(tableMock.getName()).thenReturn("KeyTable");
-    when(omMetadataManagerMock.getKeyTable(getBucketLayout()))
+    when(omMetadataManagerMock.getKeyTable(BucketLayout.DEFAULT))

Review Comment:
   same applies here as above



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