Re: [PR] HDDS-10298. Replace estimated count with actual count while listing openkeys. [ozone]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #9043:
URL: https://github.com/apache/ozone/pull/9043#discussion_r2393273203


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1328,11 +1328,18 @@ private boolean isOpenMultipartKey(OmKeyInfo openKeyInfo, String openDbKey)
     return getMultipartInfoTable().isExist(multipartInfoDbKey);
   }
 
-  @Override
-  public long getTotalOpenKeyCount() throws IOException {
-    // Get an estimated key count of OpenKeyTable + OpenFileTable
-    return openKeyTable.getEstimatedKeyCount()
-        + openFileTable.getEstimatedKeyCount();
+  private long getTotalOpenKeyCount(BucketLayout bucketLayout, String prefix) throws IOException {
+    long count = 0;
+    try (TableIterator<String, String>
+             keyValueTableIterator = getOpenKeyTable(bucketLayout).keyIterator(prefix)) {
+      while (keyValueTableIterator.hasNext()) {
+        count += 1;
+        keyValueTableIterator.next();
+      }
+    } catch (UncheckedIOException e) {
+      throw e.getCause();

Review Comment:
   Throwing e.getCause() can result in throwing a null if the UncheckedIOException doesn't have a cause. Consider throwing the original exception or checking for null cause first.
   ```suggestion
         if (e.getCause() != null) {
           throw e.getCause();
         } else {
           throw e;
         }
   ```



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1328,11 +1328,18 @@ private boolean isOpenMultipartKey(OmKeyInfo openKeyInfo, String openDbKey)
     return getMultipartInfoTable().isExist(multipartInfoDbKey);
   }
 
-  @Override
-  public long getTotalOpenKeyCount() throws IOException {
-    // Get an estimated key count of OpenKeyTable + OpenFileTable
-    return openKeyTable.getEstimatedKeyCount()
-        + openFileTable.getEstimatedKeyCount();
+  private long getTotalOpenKeyCount(BucketLayout bucketLayout, String prefix) throws IOException {
+    long count = 0;
+    try (TableIterator<String, String>
+             keyValueTableIterator = getOpenKeyTable(bucketLayout).keyIterator(prefix)) {
+      while (keyValueTableIterator.hasNext()) {
+        count += 1;

Review Comment:
   [nitpick] Use pre-increment (++count) instead of addition assignment for better readability and potential performance benefits when incrementing by 1.
   ```suggestion
           ++count;
   ```



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