[GitHub] [accumulo] keith-turner commented on a diff in pull request #2792: closes #1377 - ensure all tables are checked ...

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

keith-turner commented on code in PR #2792:
URL: https://github.com/apache/accumulo/pull/2792#discussion_r969605447


##########
server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java:
##########
@@ -172,7 +177,19 @@ public Stream<Reference> getReferences() {
 
   @Override
   public Set<TableId> getTableIDs() {
-    return context.getTableIdToNameMap().keySet();
+    final String tablesPath = context.getZooKeeperRoot() + Constants.ZTABLES;
+    final ZooReader zr = context.getZooReader();
+    final Set<TableId> tids = new HashSet<>();
+    while (true) {
+      try {
+        zr.sync(tablesPath);
+        zr.getChildren(tablesPath).forEach(t -> tids.add(TableId.of(t)));
+        return tids;
+      } catch (KeeperException | InterruptedException e) {
+        log.error("Error getting tables from ZooKeeper, retrying", e);
+        UtilWaitThread.sleepUninterruptibly(1, TimeUnit.SECONDS);
+      }
+    }

Review Comment:
   Maybe the new `getCandidateTableIDs()` method is not needed and this method could be modified.  Looking around at the code there is only one place this is used currently and I think this change would be good for that code.   The existing code that uses it may not be correct for metadata and root  levels (but I think luckily it may not matter).
   
   ```suggestion
       switch(level){
       case ROOT:
           return Set.of(RootTable.ID);
       case METADATA:
           return Set.of(MetadataTable.ID);
       case USER:
         final String tablesPath = context.getZooKeeperRoot() + Constants.ZTABLES;
         final ZooReader zr = context.getZooReader();
         final Set<TableId> tids = new HashSet<>();
         while (true) {
           try {
             zr.sync(tablesPath);
             zr.getChildren(tablesPath).forEach(t -> tids.add(TableId.of(t)));
             return tids;
           } catch (KeeperException | InterruptedException e) {
             log.error("Error getting tables from ZooKeeper, retrying", e);
             UtilWaitThread.sleepUninterruptibly(1, TimeUnit.SECONDS);
           }
         }
       default:
          throw new IllegalArgumentException("unknown level "+level);
       }
   ```



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org