adoroszlai commented on code in PR #8069:
URL: https://github.com/apache/ozone/pull/8069#discussion_r1995082758


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OmTableInsightTask.java:
##########
@@ -112,20 +112,26 @@ public TaskResult reprocess(OMMetadataManager 
omMetadataManager) {
     for (String tableName : tables) {
       Table table = omMetadataManager.getTable(tableName);
 
-      try (TableIterator<String, ? extends Table.KeyValue<String, ?>> iterator
-               = table.iterator()) {
+      try {
         if (tableHandlers.containsKey(tableName)) {
-          Triple<Long, Long, Long> details =
-              tableHandlers.get(tableName).getTableSizeAndCount(iterator);
-          objectCountMap.put(getTableCountKeyFromTable(tableName),
-              details.getLeft());
-          unReplicatedSizeMap.put(
-              getUnReplicatedSizeKeyFromTable(tableName), details.getMiddle());
-          replicatedSizeMap.put(getReplicatedSizeKeyFromTable(tableName),
-              details.getRight());
+          try (TableIterator<String, ? extends Table.KeyValue<String, ?>> 
iterator
+                   = table.iterator()) {
+            Triple<Long, Long, Long> details =
+                tableHandlers.get(tableName).getTableSizeAndCount(iterator);
+            objectCountMap.put(getTableCountKeyFromTable(tableName),
+                details.getLeft());
+            unReplicatedSizeMap.put(
+                getUnReplicatedSizeKeyFromTable(tableName), 
details.getMiddle());
+            replicatedSizeMap.put(getReplicatedSizeKeyFromTable(tableName),
+                details.getRight());
+          }
         } else {
-          long count = Iterators.size(iterator);
-          objectCountMap.put(getTableCountKeyFromTable(tableName), count);
+          try (
+              TableIterator<String, ? extends Table.KeyValue<String, ?>> 
rawTableIterator
+                  = table.getRawTable().iterator()) {
+            long count = Iterators.size(rawTableIterator);

Review Comment:
   > `getEstimatedKeyCount()` doesn't provide accurate count.
   
   I know, the question was: does Recon _really_ need exact count?
   
   And _if_ it does, then:
   
   - Do not expose `Table<byte[], byte[]> getRawTable()` in the interface, as 
it breaks encapsulation.  It also doesn't match the existing interface, there 
is no mention of `byte[]` anywhere in `Table` (other than outdated comments).
   - Add `getExactKeyCount()`, similar to `getEstimatedKeyCount()`, where 
`RDBTable` has the real logic, and other implementations can delegate to the 
underlying table.
   
   This allows other users of `Table` to get count, instead of duplicating the 
iterator logic.



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