Clears unreferenced tables from TabletLocator

core/src/main/java/org/apache/accumulo/core/clientImpl/TabletLocator.java

@@ -20,6 +20,7 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -41,8 +42,10 @@
 import org.apache.accumulo.core.singletons.SingletonManager;
 import org.apache.accumulo.core.singletons.SingletonService;
 import org.apache.accumulo.core.util.Interner;
+import org.apache.accumulo.core.util.Timer;
 import org.apache.hadoop.io.Text;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
 public abstract class TabletLocator {
@@ -111,7 +114,8 @@ public boolean equals(LocatorKey lk) {
   }
 
   private static final HashMap<LocatorKey,TabletLocator> locators = new HashMap<>();
-  private static final HashMap<TableId,OfflineTabletLocatorImpl> offlineLocators = new HashMap<>();
+  private static final HashMap<LocatorKey,OfflineTabletLocatorImpl> offlineLocators =
+      new HashMap<>();
   private static boolean enabled = true;
 
   public static synchronized void clearLocators() {
@@ -138,15 +142,17 @@ static synchronized void enable() {
   public static synchronized TabletLocator getLocator(ClientContext context, TableId tableId) {
     Preconditions.checkState(enabled, "The Accumulo singleton that that tracks tablet locations is "
         + "disabled. This is likely caused by all AccumuloClients being closed or garbage collected");
+
+    clearUnusedTables(context);
+
     TableState state = context.getTableState(tableId);
+    LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
     if (state == TableState.OFFLINE) {
-      LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
       locators.remove(key);
-      return offlineLocators.computeIfAbsent(tableId,
+      return offlineLocators.computeIfAbsent(key,
           f -> new OfflineTabletLocatorImpl(context, tableId));
     } else {
-      offlineLocators.remove(tableId);
-      LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
+      offlineLocators.remove(key);
       TabletLocator tl = locators.get(key);
       if (tl == null) {
         MetadataLocationObtainer mlo = new MetadataLocationObtainer();
@@ -167,6 +173,60 @@ public static synchronized TabletLocator getLocator(ClientContext context, Table
 
   }
 
+  /**
+   * Checks if a table id is present in the cache w/o creating it.
+   */
+  @VisibleForTesting
+  public static synchronized boolean isPresent(ClientContext context, TableId tableId) {
+    LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
+    return locators.containsKey(key) || offlineLocators.containsKey(key);
+  }
+
+  private static Duration clearFrequency = Duration.ofMinutes(10);
+
+  /**
+   * Sets how often checks for unused tables are done
+   */
+  @VisibleForTesting
+  public static synchronized void setClearFrequency(Duration frequency) {
+    Preconditions.checkArgument(frequency != null && !frequency.isNegative() && !frequency.isZero(),
+        "frequency:%s", frequency);
+    clearFrequency = frequency;
+  }
+
+  private static final Timer lastClearTimer = Timer.startNew();
+
+  /**
+   * Finds and clears any tables ids in the cache that are no longer in used.
+   */
+  private static synchronized void clearUnusedTables(ClientContext context) {
+    if (lastClearTimer.hasElapsed(clearFrequency)) {
+      locators.entrySet().removeIf(entry -> {
+        LocatorKey lkey = entry.getKey();
+        TabletLocator locator = entry.getValue();
+        if (lkey.instanceId.equals(context.getInstanceID())
+            && context.getTableState(lkey.tableId) != TableState.ONLINE) {
+          locator.isValid = false;
+          locator.invalidateCache();
+          return true;
+        }
+        return false;
+      });
+      offlineLocators.entrySet().removeIf(entry -> {
+        LocatorKey lkey = entry.getKey();
+        TabletLocator locator = entry.getValue();
+        if (lkey.instanceId.equals(context.getInstanceID())
+            && context.getTableState(lkey.tableId) != TableState.OFFLINE) {
+          locator.isValid = false;
+          locator.invalidateCache();
+          return true;
+        }
+        return false;
+      });
+      lastClearTimer.restart();
+    }
+  }
+
   static {
     SingletonManager.register(new SingletonService() {
 

test/src/main/java/org/apache/accumulo/test/LocatorIT.java

@@ -19,6 +19,7 @@
 package org.apache.accumulo.test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -38,13 +39,18 @@
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.client.TableOfflineException;
 import org.apache.accumulo.core.client.admin.Locations;
+import org.apache.accumulo.core.client.admin.NewTableConfiguration;
 import org.apache.accumulo.core.client.admin.TableOperations;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.TabletLocator;
 import org.apache.accumulo.core.data.Range;
 import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.data.TabletId;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.dataImpl.TabletIdImpl;
+import org.apache.accumulo.core.manager.state.tables.TableState;
 import org.apache.accumulo.harness.AccumuloClusterHarness;
+import org.apache.accumulo.test.util.Wait;
 import org.apache.hadoop.io.Text;
 import org.junit.jupiter.api.Test;
 
@@ -132,4 +138,78 @@ public void testBasic() throws Exception {
       assertThrows(TableNotFoundException.class, () -> tableOps.locate(tableName, ranges));
     }
   }
+
+  @Test
+  public void testClearingUnused() throws Exception {
+    try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) {
+      String[] tables = getUniqueNames(4);
+      String table1 = tables[0];
+      String table2 = tables[1];
+      String table3 = tables[2];
+      String table4 = tables[3];
+
+      TableOperations tableOps = client.tableOperations();
+      tableOps.create(table1);
+      tableOps.create(table2);
+      tableOps.create(table3, new NewTableConfiguration().createOffline());
+      tableOps.create(table4, new NewTableConfiguration().createOffline());
+
+      TabletLocator.setClearFrequency(Duration.ofMillis(100));
+
+      ClientContext ctx = (ClientContext) client;
+      TableId tableId1 = ctx.getTableId(table1);
+      TableId tableId2 = ctx.getTableId(table2);
+      TableId tableId3 = ctx.getTableId(table3);
+      TableId tableId4 = ctx.getTableId(table4);
+
+      for (var tableId : List.of(tableId1, tableId2, tableId3, tableId4)) {
+        assertFalse(TabletLocator.isPresent(ctx, tableId));
+        assertNotNull(TabletLocator.getLocator(ctx, tableId));
+        assertTrue(TabletLocator.isPresent(ctx, tableId));
+      }
+
+      // Put table2 and table3 into a different state than what is in the cache
+      assertEquals(TableState.ONLINE, ctx.getTableState(tableId2));
+      assertEquals(TableState.OFFLINE, ctx.getTableState(tableId3));
+      tableOps.offline(table2, true);
+      tableOps.online(table3, true);
+      assertEquals(TableState.OFFLINE, ctx.getTableState(tableId2));
+      assertEquals(TableState.ONLINE, ctx.getTableState(tableId3));
+
+      Wait.waitFor(() -> {
+        // Accessing table1 in the cache should cause table2 and table3 to eventually be cleared
+        // because their table state does not match what was cached
+        assertNotNull(TabletLocator.getLocator(ctx, tableId1));
+        return !TabletLocator.isPresent(ctx, tableId2) && !TabletLocator.isPresent(ctx, tableId3);
+      });
+
+      assertTrue(TabletLocator.isPresent(ctx, tableId1));
+      assertTrue(TabletLocator.isPresent(ctx, tableId4));
+
+      // bring table2 and table3 back into the cache
+      for (var tableId : List.of(tableId2, tableId3)) {
+        assertFalse(TabletLocator.isPresent(ctx, tableId));
+        assertNotNull(TabletLocator.getLocator(ctx, tableId));
+        assertTrue(TabletLocator.isPresent(ctx, tableId));
+      }
+
+      tableOps.delete(table2);
+      tableOps.delete(table3);
+
+      Wait.waitFor(() -> {
+        // Accessing table4 in the cache should cause table2 and table3 to eventually be cleared
+        // because they no longer exist. This also test that online and offline tables a properly
+        // cleared from the cache.
+        assertNotNull(TabletLocator.getLocator(ctx, tableId4));
+        return !TabletLocator.isPresent(ctx, tableId2) && !TabletLocator.isPresent(ctx, tableId3);
+      });
+
+      // table1 and table4 should be left in the cache, check that online or offline tables are not
+      // removed unnecessarily.
+      assertTrue(TabletLocator.isPresent(ctx, tableId1));
+      assertTrue(TabletLocator.isPresent(ctx, tableId4));
+      assertEquals(TableState.ONLINE, ctx.getTableState(tableId1));
+      assertEquals(TableState.OFFLINE, ctx.getTableState(tableId4));
+    }
+  }
 }