Disable nagative cache in VirtualTableInfoManager

Author: brfrn169

build.gradle

@@ -43,6 +43,7 @@ subprojects {
         alloyDbJdbcConnectorVersion = '1.2.8'
         picocliVersion = '4.7.7'
         commonsTextVersion = '1.14.0'
+        caffeineVersion = '2.9.3'
         junitVersion = '5.14.1'
         commonsLangVersion = '3.20.0'
         assertjVersion = '3.27.6'

core/build.gradle

@@ -194,6 +194,7 @@ dependencies {
         exclude group: 'org.slf4j', module: 'slf4j-api'
     }
     implementation "org.apache.commons:commons-text:${commonsTextVersion}"
+    implementation "com.github.ben-manes.caffeine:caffeine:${caffeineVersion}"
     testImplementation platform("org.junit:junit-bom:${junitVersion}")
     testImplementation 'org.junit.jupiter:junit-jupiter'
     testRuntimeOnly 'org.junit.platform:junit-platform-launcher'

core/src/main/java/com/scalar/db/common/TableMetadataManager.java

@@ -1,6 +1,5 @@
 package com.scalar.db.common;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -14,7 +13,6 @@
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 
 /** A class that manages and caches table metadata */
@@ -54,7 +52,6 @@ public Optional<TableMetadata> load(@Nonnull TableKey key) throws Exception {
    * @return a table metadata. null if the table is not found.
    * @throws ExecutionException if the operation fails
    */
-  @Nullable
   public TableMetadata getTableMetadata(Operation operation) throws ExecutionException {
     if (!operation.forNamespace().isPresent() || !operation.forTable().isPresent()) {
       throw new IllegalArgumentException(
@@ -64,14 +61,13 @@ public TableMetadata getTableMetadata(Operation operation) throws ExecutionExcep
   }
 
   /**
-   * Returns a table metadata corresponding to the specified namespace and table.
+   * Returns a table metadata corresponding to the specified operation.
    *
    * @param namespace a namespace to retrieve
    * @param table a table to retrieve
    * @return a table metadata. null if the table is not found.
    * @throws ExecutionException if the operation fails
    */
-  @Nullable
   public TableMetadata getTableMetadata(String namespace, String table) throws ExecutionException {
     try {
       TableKey key = new TableKey(namespace, table);
@@ -80,12 +76,11 @@ public TableMetadata getTableMetadata(String namespace, String table) throws Exe
       throw new ExecutionException(
           CoreError.GETTING_TABLE_METADATA_FAILED.buildMessage(
               ScalarDbUtils.getFullTableName(namespace, table)),
-          e.getCause());
+          e);
     }
   }
 
-  @VisibleForTesting
-  static class TableKey {
+  public static class TableKey {
     public final String namespace;
     public final String table;
 

core/src/main/java/com/scalar/db/common/VirtualTableInfoManager.java

@@ -1,41 +1,32 @@
 package com.scalar.db.common;
 
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
 import com.scalar.db.api.DistributedStorageAdmin;
 import com.scalar.db.api.Operation;
 import com.scalar.db.api.VirtualTableInfo;
 import com.scalar.db.exception.storage.ExecutionException;
 import com.scalar.db.util.ScalarDbUtils;
 import java.util.Objects;
-import java.util.Optional;
+import java.util.concurrent.CompletionException;
 import java.util.concurrent.TimeUnit;
-import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 
 /** A class that manages and caches virtual table information */
 @ThreadSafe
 public class VirtualTableInfoManager {
 
-  private final LoadingCache<TableKey, Optional<VirtualTableInfo>> virtualTableInfoCache;
+  private final LoadingCache<TableKey, VirtualTableInfo> virtualTableInfoCache;
 
   public VirtualTableInfoManager(DistributedStorageAdmin admin, long cacheExpirationTimeSecs) {
-    CacheBuilder<Object, Object> builder = CacheBuilder.newBuilder();
+    Caffeine<Object, Object> builder = Caffeine.newBuilder();
     if (cacheExpirationTimeSecs >= 0) {
       builder.expireAfterWrite(cacheExpirationTimeSecs, TimeUnit.SECONDS);
     }
     virtualTableInfoCache =
-        builder.build(
-            new CacheLoader<TableKey, Optional<VirtualTableInfo>>() {
-              @Nonnull
-              @Override
-              public Optional<VirtualTableInfo> load(@Nonnull TableKey key) throws Exception {
-                return admin.getVirtualTableInfo(key.namespace, key.table);
-              }
-            });
+        builder.build(key -> admin.getVirtualTableInfo(key.namespace, key.table).orElse(null));
   }
 
   /**
@@ -69,13 +60,8 @@ public VirtualTableInfo getVirtualTableInfo(String namespace, String table)
       throws ExecutionException {
     try {
       TableKey key = new TableKey(namespace, table);
-      return virtualTableInfoCache.get(key).orElse(null);
-    } catch (java.util.concurrent.ExecutionException
-        | com.google.common.util.concurrent.UncheckedExecutionException e) {
-      if (e.getCause() instanceof RuntimeException) {
-        throw (RuntimeException) e.getCause();
-      }
-
+      return virtualTableInfoCache.get(key);
+    } catch (CompletionException e) {
       throw new ExecutionException(
           CoreError.GETTING_VIRTUAL_TABLE_INFO_FAILED.buildMessage(
               ScalarDbUtils.getFullTableName(namespace, table)),

core/src/test/java/com/scalar/db/common/VirtualTableInfoManagerTest.java

@@ -120,35 +120,6 @@ public void getVirtualTableInfo_VirtualTableDoesNotExist_ShouldReturnNull() thro
     verify(admin).getVirtualTableInfo("ns", "table");
   }
 
-  @Test
-  public void getVirtualTableInfo_AdminThrowsRuntimeException_ShouldThrowRuntimeException()
-      throws Exception {
-    // Arrange
-    manager = new VirtualTableInfoManager(admin, -1);
-    when(admin.getVirtualTableInfo("ns", "table"))
-        .thenThrow(new IllegalArgumentException("Table does not exist"));
-
-    // Act Assert
-    assertThatThrownBy(() -> manager.getVirtualTableInfo("ns", "table"))
-        .isInstanceOf(IllegalArgumentException.class)
-        .hasMessageContaining("Table does not exist");
-  }
-
-  @Test
-  public void getVirtualTableInfo_AdminThrowsExecutionException_ShouldWrapInExecutionException()
-      throws Exception {
-    // Arrange
-    manager = new VirtualTableInfoManager(admin, -1);
-    when(admin.getVirtualTableInfo("ns", "table"))
-        .thenThrow(new ExecutionException("Storage execution error"));
-
-    // Act Assert
-    assertThatThrownBy(() -> manager.getVirtualTableInfo("ns", "table"))
-        .isInstanceOf(ExecutionException.class)
-        .hasMessageContaining("Getting the virtual table information failed")
-        .hasMessageContaining("ns.table");
-  }
-
   @Test
   public void getVirtualTableInfo_CalledMultipleTimes_ShouldUseCache() throws Exception {
     // Arrange
@@ -185,6 +156,51 @@ public void getVirtualTableInfo_WithCacheExpiration_ShouldExpireCache() throws E
     verify(admin, times(2)).getVirtualTableInfo("ns", "table");
   }
 
+  @Test
+  public void getVirtualTableInfo_VirtualTableDoesNotExist_ShouldNotCacheNullResult()
+      throws Exception {
+    // Arrange
+    manager = new VirtualTableInfoManager(admin, -1);
+    when(admin.getVirtualTableInfo("ns", "table")).thenReturn(Optional.empty());
+
+    // Act
+    manager.getVirtualTableInfo("ns", "table");
+    manager.getVirtualTableInfo("ns", "table");
+    manager.getVirtualTableInfo("ns", "table");
+
+    // Assert - admin should be called multiple times because null is not cached
+    verify(admin, times(3)).getVirtualTableInfo("ns", "table");
+  }
+
+  @Test
+  public void getVirtualTableInfo_AdminThrowsRuntimeException_ShouldThrowRuntimeException()
+      throws Exception {
+    // Arrange
+    manager = new VirtualTableInfoManager(admin, -1);
+    when(admin.getVirtualTableInfo("ns", "table"))
+        .thenThrow(new IllegalArgumentException("Table does not exist"));
+
+    // Act Assert
+    assertThatThrownBy(() -> manager.getVirtualTableInfo("ns", "table"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Table does not exist");
+  }
+
+  @Test
+  public void getVirtualTableInfo_AdminThrowsExecutionException_ShouldWrapInExecutionException()
+      throws Exception {
+    // Arrange
+    manager = new VirtualTableInfoManager(admin, -1);
+    when(admin.getVirtualTableInfo("ns", "table"))
+        .thenThrow(new ExecutionException("Storage execution error"));
+
+    // Act Assert
+    assertThatThrownBy(() -> manager.getVirtualTableInfo("ns", "table"))
+        .isInstanceOf(ExecutionException.class)
+        .hasMessageContaining("Getting the virtual table information failed")
+        .hasMessageContaining("ns.table");
+  }
+
   private VirtualTableInfo createVirtualTableInfo() {
     return new VirtualTableInfo() {
       @Override