[GitHub] [ozone] rakeshadr commented on a change in pull request #2561: HDDS-5636. Use bucket layout specific DB table in OmMetadataManager:getOpenKeyTable().

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

rakeshadr commented on a change in pull request #2561:
URL: https://github.com/apache/ozone/pull/2561#discussion_r711898329



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java
##########
@@ -1492,4 +1493,8 @@ private RequestContext currentUserReads() throws IOException {
         .setAclType(ACLIdentityType.USER)
         .build();
   }
+
+  public BucketLayout getBucketLayout() {

Review comment:
       @aryangupta1998, Adding two points:
   Point-1) I agree BucketLayout.DEFAULT is correct but I'd prefer to explicitly to use `BucketLayout.LEGACY` instead of using BucketLayout.DEFAULT. My reasoning is, in future if someone changes DEFAULT value to something else, then all the test cases has to be revisited. As we know all these test cases are legacy fashion and it can be defined to `BucketLayout.LEGACY` 
   
   Point-2) Please use correct visibility either public or private. Please check all the usages of `public BucketLayout getBucketLayout() {` method in different test classes.
   
   Also, if the above method is overridden in inherited class then please use annotation `@override` on the method.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -300,12 +301,11 @@ protected OmMetadataManagerImpl() {
     return deletedDirTable;
   }
 
-  @Override
-  public Table<String, OmKeyInfo> getOpenKeyTable() {
-    if (OzoneManagerRatisUtils.isBucketFSOptimized()) {
-      return openFileTable;
+  public Table getOpenKeyTable(BucketLayout bucketLayout) {
+    if (bucketLayout.equals(BucketLayout.OBJECT_STORE)) {

Review comment:
       @aryangupta1998 thanks for the patch. Like we discussed offline to fix the test failure, could you modify the condition to include the LEGACY case.
   
      ```
    if (bucketLayout.equals(BucketLayout.FILE_SYSTEM_OPTIMIZED)) {
         return openFileTable;
       }
       return openKeyTable;
   ```

##########
File path: hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
##########
@@ -311,7 +312,14 @@ boolean recoverTrash(String volumeName, String bucketName,
    *
    * @return Table.
    */
-  Table<String, OmKeyInfo> getOpenKeyTable();
+  //Table<String, OmKeyInfo> getOpenKeyTable();

Review comment:
       Please remove the commented line.




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