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


##########
hadoop-ozone/dist/src/main/smoketest/recon/recon-nssummary.robot:
##########
@@ -14,7 +14,7 @@
 # limitations under the License.
 
 *** Settings ***
-Documentation       Smoke test for Recon Namespace Summary Endpoint for FSO 
buckets.
+Documentation       Smoke test for Recon Namespace Summary Endpoint for FSO 
and Legacy buckets.

Review Comment:
   If you are also testing the `LEGACY` buckets here why did you added a 
`${BUCKET_LAYOUT}` variable below that is set to `FILE_SYSTEM_OPTIMIZED`? Will 
you add test cases in this robot test file for `LEGACY` too? 



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/handlers/LegacyBucketHandler.java:
##########
@@ -0,0 +1,318 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.recon.api.handlers;
+
+import org.apache.hadoop.hdds.scm.server.OzoneStorageContainerManager;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.recon.api.types.DUResponse;
+import org.apache.hadoop.ozone.recon.api.types.EntityType;
+import org.apache.hadoop.ozone.recon.api.types.NSSummary;
+import org.apache.hadoop.ozone.recon.recovery.ReconOMMetadataManager;
+import org.apache.hadoop.ozone.recon.spi.ReconNamespaceSummaryManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+
+/**
+ * Class for handling Legacy buckets.
+ */
+public class LegacyBucketHandler extends BucketHandler {
+
+  private static final Logger LOG = LoggerFactory.getLogger(
+      LegacyBucketHandler.class);
+
+  private final String vol;
+  private final String bucket;
+  private final OmBucketInfo omBucketInfo;
+
+  public LegacyBucketHandler(
+      ReconNamespaceSummaryManager reconNamespaceSummaryManager,
+      ReconOMMetadataManager omMetadataManager,
+      OzoneStorageContainerManager reconSCM,
+      OmBucketInfo bucketInfo) {
+    super(reconNamespaceSummaryManager, omMetadataManager,
+        reconSCM);
+    this.omBucketInfo = bucketInfo;
+    this.vol = omBucketInfo.getVolumeName();
+    this.bucket = omBucketInfo.getBucketName();
+  }
+
+  /**
+   * Helper function to check if a path is a directory, key, or invalid.
+   * @param keyName key name
+   * @return DIRECTORY, KEY, or UNKNOWN
+   * @throws IOException
+   */
+  @Override
+  public EntityType determineKeyPath(String keyName)
+      throws IOException {
+
+    String filename = OzoneFSUtils.removeTrailingSlashIfNeeded(keyName);
+    // For example, /vol1/buck1/a/b/c/d/e/file1.txt
+    // Look in the KeyTable for the key path,
+    // if the first one we seek to is the same as the seek key,
+    // it is a key;
+    // if it is the seekKey with a trailing slash, it is a directory
+    // else it is unknown
+    String key = OM_KEY_PREFIX + vol +
+        OM_KEY_PREFIX + bucket +
+        OM_KEY_PREFIX + filename;
+
+    Table<String, OmKeyInfo> keyTable = getKeyTable();
+
+    TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iterator = keyTable.iterator();
+
+    iterator.seek(key);
+    if (iterator.hasNext()) {
+      Table.KeyValue<String, OmKeyInfo> kv = iterator.next();
+      String dbKey = kv.getKey();
+      if (dbKey.equals(key)) {
+        return EntityType.KEY;
+      }
+      if (dbKey.equals(key + OM_KEY_PREFIX)) {
+        return EntityType.DIRECTORY;
+      }
+    }
+    return EntityType.UNKNOWN;
+  }
+
+  // KeyTable's key is in the format of "vol/bucket/keyName"
+  // Make use of RocksDB's order to seek to the prefix and avoid full iteration
+  @Override
+  public long calculateDUUnderObject(long parentId)

Review Comment:
   Could you expand this method's documentation here? 



##########
hadoop-ozone/dist/src/main/smoketest/recon/recon-nssummary.robot:
##########
@@ -131,7 +132,8 @@ Check Recon Namespace Summary Key
     Wait For Summary      ${SUMMARY_URL}?path=/${VOLUME}/${BUCKET}/file1   KEY
 
 Check Recon Namespace Summary Directory
-    Wait For Summary      ${SUMMARY_URL}?path=/${VOLUME}/${BUCKET}/dir1/dir2   
DIRECTORY
+    Run Keyword If    '${BUCKET_LAYOUT}' == 'LEGACY'                    Wait 
For Summary      ${SUMMARY_URL}?path=/${VOLUME}/${BUCKET}/dir1/dir2/   DIRECTORY

Review Comment:
   If the `${BUCKET_LAYOUT}` is set to `FILE_SYSTEM_OPTIMIZED` why are we 
checking if it is `LEGACY`? Is there a place where we change it? What kind of 
response do we expect here?



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