Remove unused caches in BigQueryClient

They were never written to, so always empty. This also removes `bigquery.case-insensitive-name-matching.cache-ttl` config property, which was accepted, but didn't do anything.
trinodb · Jan 21, 2022 · 10c4756 · 10c4756
1 parent affdeb9
commit 10c4756
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 50 deletions.
diff --git a/docs/src/main/sphinx/connector/bigquery.rst b/docs/src/main/sphinx/connector/bigquery.rst
@@ -130,11 +130,6 @@ Property                                              Description
 ``bigquery.credentials-key``                          The base64 encoded credentials key                             None. See the `requirements <#requirements>`_ section.
 ``bigquery.credentials-file``                         The path to the JSON credentials file                          None. See the `requirements <#requirements>`_ section.
 ``bigquery.case-insensitive-name-matching``           Match dataset and table names case-insensitively               ``false``
-``bigquery.case-insensitive-name-matching.cache-ttl`` Duration for which remote dataset and table names will be      ``1m``
-                                                      cached. Higher values reduce the number of API calls to
-                                                      BigQuery but can cause newly created dataset or tables to not
-                                                      be visible until the configured duration. Set to ``0ms`` to
-                                                      disable the cache.
 ===================================================== ============================================================== ======================================================
 
 Data types

diff --git a/plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java b/plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java
@@ -28,8 +28,6 @@
 import com.google.cloud.bigquery.TableInfo;
 import com.google.cloud.bigquery.TableResult;
 import com.google.cloud.http.BaseHttpServiceException;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.ImmutableSet;
 import io.airlift.log.Logger;
 import io.airlift.units.Duration;
@@ -54,7 +52,6 @@
 import static java.lang.String.format;
 import static java.util.Locale.ENGLISH;
 import static java.util.Objects.requireNonNull;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.stream.Collectors.joining;
 
 public class BigQueryClient
@@ -64,21 +61,12 @@ public class BigQueryClient
     private final BigQuery bigQuery;
     private final ViewMaterializationCache materializationCache;
     private final boolean caseInsensitiveNameMatching;
-    private final Cache<String, Optional<RemoteDatabaseObject>> remoteDatasets;
-    private final Cache<TableId, Optional<RemoteDatabaseObject>> remoteTables;
 
     public BigQueryClient(BigQuery bigQuery, BigQueryConfig config, ViewMaterializationCache materializationCache)
     {
         this.bigQuery = requireNonNull(bigQuery, "bigQuery is null");
         this.materializationCache = requireNonNull(materializationCache, "materializationCache is null");
-
-        Duration caseInsensitiveNameMatchingCacheTtl = requireNonNull(config.getCaseInsensitiveNameMatchingCacheTtl(), "caseInsensitiveNameMatchingCacheTtl is null");
-
         this.caseInsensitiveNameMatching = config.isCaseInsensitiveNameMatching();
-        CacheBuilder<Object, Object> remoteNamesCacheBuilder = CacheBuilder.newBuilder()
-                .expireAfterWrite(caseInsensitiveNameMatchingCacheTtl.toMillis(), MILLISECONDS);
-        this.remoteDatasets = remoteNamesCacheBuilder.build();
-        this.remoteTables = remoteNamesCacheBuilder.build();
     }
 
     public Optional<RemoteDatabaseObject> toRemoteDataset(String projectId, String datasetName)
@@ -90,12 +78,6 @@ public Optional<RemoteDatabaseObject> toRemoteDataset(String projectId, String d
             return Optional.of(RemoteDatabaseObject.of(datasetName));
         }
 
-        Optional<RemoteDatabaseObject> remoteDataset = remoteDatasets.getIfPresent(datasetName);
-        if (remoteDataset != null) {
-            return remoteDataset;
-        }
-
-        // cache miss, reload the cache
         Map<String, Optional<RemoteDatabaseObject>> mapping = new HashMap<>();
         for (Dataset dataset : listDatasets(projectId)) {
             mapping.merge(
@@ -104,8 +86,8 @@ public Optional<RemoteDatabaseObject> toRemoteDataset(String projectId, String d
                     (currentValue, collision) -> currentValue.map(current -> current.registerCollision(collision.get().getOnlyRemoteName())));
         }
 
-        // explicitly cache the information if the requested dataset doesn't exist
         if (!mapping.containsKey(datasetName)) {
+            // dataset doesn't exist
             mapping.put(datasetName, Optional.empty());
         }
 
@@ -134,12 +116,7 @@ private Optional<RemoteDatabaseObject> toRemoteTable(String projectId, String re
         }
 
         TableId cacheKey = TableId.of(projectId, remoteDatasetName, tableName);
-        Optional<RemoteDatabaseObject> remoteTable = remoteTables.getIfPresent(cacheKey);
-        if (remoteTable != null) {
-            return remoteTable;
-        }
 
-        // cache miss, reload the cache
         Map<TableId, Optional<RemoteDatabaseObject>> mapping = new HashMap<>();
         for (Table table : tables.get()) {
             mapping.merge(
@@ -148,8 +125,8 @@ private Optional<RemoteDatabaseObject> toRemoteTable(String projectId, String re
                     (currentValue, collision) -> currentValue.map(current -> current.registerCollision(collision.get().getOnlyRemoteName())));
         }
 
-        // explicitly cache the information if the requested table doesn't exist
         if (!mapping.containsKey(cacheKey)) {
+            // table doesn't exist
             mapping.put(cacheKey, Optional.empty());
         }
 

diff --git a/plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryConfig.java b/plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryConfig.java
@@ -16,6 +16,7 @@
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
 import io.airlift.configuration.ConfigHidden;
+import io.airlift.configuration.DefunctConfig;
 import io.airlift.units.Duration;
 import io.airlift.units.MinDuration;
 
@@ -27,6 +28,7 @@
 import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MINUTES;
 
+@DefunctConfig("bigquery.case-insensitive-name-matching.cache-ttl")
 public class BigQueryConfig
 {
     public static final int DEFAULT_MAX_READ_ROWS_RETRIES = 3;
@@ -40,7 +42,6 @@ public class BigQueryConfig
     private Optional<String> viewMaterializationDataset = Optional.empty();
     private int maxReadRowsRetries = DEFAULT_MAX_READ_ROWS_RETRIES;
     private boolean caseInsensitiveNameMatching;
-    private Duration caseInsensitiveNameMatchingCacheTtl = new Duration(1, MINUTES);
     private Duration viewsCacheTtl = new Duration(15, MINUTES);
     private Duration serviceCacheTtl = new Duration(3, MINUTES);
 
@@ -155,21 +156,6 @@ public BigQueryConfig setCaseInsensitiveNameMatching(boolean caseInsensitiveName
         return this;
     }
 
-    @NotNull
-    @MinDuration("0ms")
-    public Duration getCaseInsensitiveNameMatchingCacheTtl()
-    {
-        return caseInsensitiveNameMatchingCacheTtl;
-    }
-
-    @Config("bigquery.case-insensitive-name-matching.cache-ttl")
-    @ConfigDescription("Duration for which remote dataset and table names will be cached")
-    public BigQueryConfig setCaseInsensitiveNameMatchingCacheTtl(Duration caseInsensitiveNameMatchingCacheTtl)
-    {
-        this.caseInsensitiveNameMatchingCacheTtl = caseInsensitiveNameMatchingCacheTtl;
-        return this;
-    }
-
     @NotNull
     @MinDuration("0m")
     public Duration getViewsCacheTtl()

diff --git a/plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryConfig.java b/plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryConfig.java
@@ -24,7 +24,6 @@
 import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults;
 import static java.util.concurrent.TimeUnit.DAYS;
 import static java.util.concurrent.TimeUnit.MINUTES;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 public class TestBigQueryConfig
 {
@@ -39,7 +38,6 @@ public void testDefaults()
                 .setViewMaterializationDataset(null)
                 .setMaxReadRowsRetries(3)
                 .setCaseInsensitiveNameMatching(false)
-                .setCaseInsensitiveNameMatchingCacheTtl(new Duration(1, MINUTES))
                 .setViewsCacheTtl(new Duration(15, MINUTES))
                 .setServiceCacheTtl(new Duration(3, MINUTES))
                 .setViewsEnabled(false));
@@ -57,7 +55,6 @@ public void testExplicitPropertyMappingsWithCredentialsKey()
                 .put("bigquery.view-materialization-dataset", "vmdataset")
                 .put("bigquery.max-read-rows-retries", "10")
                 .put("bigquery.case-insensitive-name-matching", "true")
-                .put("bigquery.case-insensitive-name-matching.cache-ttl", "1s")
                 .put("bigquery.views-cache-ttl", "1m")
                 .put("bigquery.service-cache-ttl", "10d")
                 .buildOrThrow();
@@ -71,7 +68,6 @@ public void testExplicitPropertyMappingsWithCredentialsKey()
                 .setViewMaterializationDataset("vmdataset")
                 .setMaxReadRowsRetries(10)
                 .setCaseInsensitiveNameMatching(true)
-                .setCaseInsensitiveNameMatchingCacheTtl(new Duration(1, SECONDS))
                 .setViewsCacheTtl(new Duration(1, MINUTES))
                 .setServiceCacheTtl(new Duration(10, DAYS));