diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcConfig.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcConfig.java index be5106dcf54a..44efaa48390d 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcConfig.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/BaseJdbcConfig.java @@ -29,24 +29,25 @@ import static com.google.common.base.Strings.nullToEmpty; import static jakarta.validation.constraints.Pattern.Flag.CASE_INSENSITIVE; -import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; public class BaseJdbcConfig { - public static final String METADATA_CACHE_TTL = "metadata.cache-ttl"; - public static final String METADATA_SCHEMAS_CACHE_TTL = "metadata.schemas.cache-ttl"; - public static final String METADATA_TABLES_CACHE_TTL = "metadata.tables.cache-ttl"; - public static final String METADATA_CACHE_MAXIMUM_SIZE = "metadata.cache-maximum-size"; + private static final String METADATA_CACHE_TTL = "metadata.cache-ttl"; + private static final String METADATA_SCHEMAS_CACHE_TTL = "metadata.schemas.cache-ttl"; + private static final String METADATA_TABLES_CACHE_TTL = "metadata.tables.cache-ttl"; + private static final String METADATA_STATISTICS_CACHE_TTL = "metadata.statistics.cache-ttl"; + private static final String METADATA_CACHE_MAXIMUM_SIZE = "metadata.cache-maximum-size"; + private static final long DEFAULT_METADATA_CACHE_SIZE = 10000; private String connectionUrl; private Set jdbcTypesMappedToVarchar = ImmutableSet.of(); - public static final Duration CACHING_DISABLED = new Duration(0, MILLISECONDS); - private Duration metadataCacheTtl = CACHING_DISABLED; + private Duration metadataCacheTtl = new Duration(0, SECONDS); private Optional schemaNamesCacheTtl = Optional.empty(); private Optional tableNamesCacheTtl = Optional.empty(); + private Optional statisticsCacheTtl = Optional.empty(); private boolean cacheMissing; - public static final long DEFAULT_METADATA_CACHE_SIZE = 10000; - private long cacheMaximumSize = DEFAULT_METADATA_CACHE_SIZE; + private Optional cacheMaximumSize = Optional.empty(); @NotNull // Some drivers match case insensitive in Driver.acceptURL @@ -118,6 +119,20 @@ public BaseJdbcConfig setTableNamesCacheTtl(Duration tableNamesCacheTtl) return this; } + @NotNull + public Duration getStatisticsCacheTtl() + { + return statisticsCacheTtl.orElse(metadataCacheTtl); + } + + @Config(METADATA_STATISTICS_CACHE_TTL) + @ConfigDescription("Determines how long table statistics information will be cached") + public BaseJdbcConfig setStatisticsCacheTtl(Duration statisticsCacheTtl) + { + this.statisticsCacheTtl = Optional.ofNullable(statisticsCacheTtl); + return this; + } + public boolean isCacheMissing() { return cacheMissing; @@ -134,32 +149,34 @@ public BaseJdbcConfig setCacheMissing(boolean cacheMissing) @Min(1) public long getCacheMaximumSize() { - return cacheMaximumSize; + return cacheMaximumSize.orElse(DEFAULT_METADATA_CACHE_SIZE); } @Config(METADATA_CACHE_MAXIMUM_SIZE) @ConfigDescription("Maximum number of objects stored in the metadata cache") public BaseJdbcConfig setCacheMaximumSize(long cacheMaximumSize) { - this.cacheMaximumSize = cacheMaximumSize; + this.cacheMaximumSize = Optional.of(cacheMaximumSize); return this; } - @AssertTrue(message = METADATA_CACHE_TTL + " must be set to a non-zero value when " + METADATA_CACHE_MAXIMUM_SIZE + " is set") + @AssertTrue(message = METADATA_CACHE_TTL + " or " + METADATA_STATISTICS_CACHE_TTL + " must be set to a non-zero value when " + METADATA_CACHE_MAXIMUM_SIZE + " is set") public boolean isCacheMaximumSizeConsistent() { - return !metadataCacheTtl.equals(CACHING_DISABLED) || cacheMaximumSize == BaseJdbcConfig.DEFAULT_METADATA_CACHE_SIZE; + return !metadataCacheTtl.isZero() || + (statisticsCacheTtl.isPresent() && !statisticsCacheTtl.get().isZero()) || + cacheMaximumSize.isEmpty(); } @AssertTrue(message = METADATA_SCHEMAS_CACHE_TTL + " must not be set when " + METADATA_CACHE_TTL + " is not set") public boolean isSchemaNamesCacheTtlConsistent() { - return !metadataCacheTtl.equals(CACHING_DISABLED) || schemaNamesCacheTtl.isEmpty(); + return !metadataCacheTtl.isZero() || schemaNamesCacheTtl.isEmpty(); } @AssertTrue(message = METADATA_TABLES_CACHE_TTL + " must not be set when " + METADATA_CACHE_TTL + " is not set") public boolean isTableNamesCacheTtlConsistent() { - return !metadataCacheTtl.equals(CACHING_DISABLED) || tableNamesCacheTtl.isEmpty(); + return !metadataCacheTtl.isZero() || tableNamesCacheTtl.isEmpty(); } } diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java index 232632c7dd5b..1a2ed089609f 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/CachingJdbcClient.java @@ -14,6 +14,7 @@ package io.trino.plugin.jdbc; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Ticker; import com.google.common.cache.Cache; import com.google.common.cache.CacheStats; import com.google.common.collect.ImmutableMap; @@ -66,7 +67,6 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static io.trino.cache.CacheUtils.invalidateAllIf; -import static io.trino.plugin.jdbc.BaseJdbcConfig.CACHING_DISABLED; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -97,41 +97,27 @@ public CachingJdbcClient( BaseJdbcConfig config) { this( + Ticker.systemTicker(), delegate, sessionPropertiesProviders, identityMapping, config.getMetadataCacheTtl(), config.getSchemaNamesCacheTtl(), config.getTableNamesCacheTtl(), + config.getStatisticsCacheTtl(), config.isCacheMissing(), config.getCacheMaximumSize()); } public CachingJdbcClient( - JdbcClient delegate, - Set sessionPropertiesProviders, - IdentityCacheMapping identityMapping, - Duration metadataCachingTtl, - boolean cacheMissing, - long cacheMaximumSize) - { - this(delegate, - sessionPropertiesProviders, - identityMapping, - metadataCachingTtl, - metadataCachingTtl, - metadataCachingTtl, - cacheMissing, - cacheMaximumSize); - } - - public CachingJdbcClient( + Ticker ticker, JdbcClient delegate, Set sessionPropertiesProviders, IdentityCacheMapping identityMapping, Duration metadataCachingTtl, Duration schemaNamesCachingTtl, Duration tableNamesCachingTtl, + Duration statisticsCachingTtl, boolean cacheMissing, long cacheMaximumSize) { @@ -142,23 +128,19 @@ public CachingJdbcClient( this.cacheMissing = cacheMissing; this.identityMapping = requireNonNull(identityMapping, "identityMapping is null"); - long cacheSize = metadataCachingTtl.equals(CACHING_DISABLED) - // Disables the cache entirely - ? 0 - : cacheMaximumSize; - - schemaNamesCache = buildCache(cacheSize, schemaNamesCachingTtl); - tableNamesCache = buildCache(cacheSize, tableNamesCachingTtl); - tableHandlesByNameCache = buildCache(cacheSize, metadataCachingTtl); - tableHandlesByQueryCache = buildCache(cacheSize, metadataCachingTtl); - procedureHandlesByQueryCache = buildCache(cacheSize, metadataCachingTtl); - columnsCache = buildCache(cacheSize, metadataCachingTtl); - statisticsCache = buildCache(cacheSize, metadataCachingTtl); + schemaNamesCache = buildCache(ticker, cacheMaximumSize, schemaNamesCachingTtl); + tableNamesCache = buildCache(ticker, cacheMaximumSize, tableNamesCachingTtl); + tableHandlesByNameCache = buildCache(ticker, cacheMaximumSize, metadataCachingTtl); + tableHandlesByQueryCache = buildCache(ticker, cacheMaximumSize, metadataCachingTtl); + procedureHandlesByQueryCache = buildCache(ticker, cacheMaximumSize, metadataCachingTtl); + columnsCache = buildCache(ticker, cacheMaximumSize, metadataCachingTtl); + statisticsCache = buildCache(ticker, cacheMaximumSize, statisticsCachingTtl); } - private static Cache buildCache(long cacheSize, Duration cachingTtl) + private static Cache buildCache(Ticker ticker, long cacheSize, Duration cachingTtl) { return EvictableCacheBuilder.newBuilder() + .ticker(ticker) .maximumSize(cacheSize) .expireAfterWrite(cachingTtl.toMillis(), MILLISECONDS) .shareNothingWhenDisabled() diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java index 76b1a91dc9dc..29078ca73bf2 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadataFactory.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.jdbc; +import com.google.common.base.Ticker; import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import io.airlift.units.Duration; @@ -41,12 +42,16 @@ public JdbcMetadata create(JdbcTransactionHandle transaction) // Session stays the same per transaction, therefore session properties don't need to // be a part of cache keys in CachingJdbcClient. return create(new CachingJdbcClient( - jdbcClient, - Set.of(), - new SingletonIdentityCacheMapping(), - new Duration(1, DAYS), - true, - Integer.MAX_VALUE)); + Ticker.systemTicker(), + jdbcClient, + Set.of(), + new SingletonIdentityCacheMapping(), + new Duration(1, DAYS), + new Duration(1, DAYS), + new Duration(1, DAYS), + new Duration(1, DAYS), + true, + Integer.MAX_VALUE)); } protected JdbcMetadata create(JdbcClient transactionCachingJdbcClient) diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestBaseJdbcConfig.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestBaseJdbcConfig.java index 3f5d200bf650..a8dfaba46f99 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestBaseJdbcConfig.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestBaseJdbcConfig.java @@ -26,15 +26,13 @@ import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults; import static io.airlift.testing.ValidationAssertions.assertFailsValidation; import static io.airlift.testing.ValidationAssertions.assertValidates; -import static java.util.concurrent.TimeUnit.MINUTES; +import static io.airlift.units.Duration.ZERO; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; public class TestBaseJdbcConfig { - private static final Duration ZERO = Duration.succinctDuration(0, MINUTES); - @Test public void testDefaults() { @@ -44,6 +42,7 @@ public void testDefaults() .setMetadataCacheTtl(ZERO) .setSchemaNamesCacheTtl(null) .setTableNamesCacheTtl(null) + .setStatisticsCacheTtl(null) .setCacheMissing(false) .setCacheMaximumSize(10000)); } @@ -57,6 +56,7 @@ public void testExplicitPropertyMappings() .put("metadata.cache-ttl", "1s") .put("metadata.schemas.cache-ttl", "2s") .put("metadata.tables.cache-ttl", "3s") + .put("metadata.statistics.cache-ttl", "7s") .put("metadata.cache-missing", "true") .put("metadata.cache-maximum-size", "5000") .buildOrThrow(); @@ -67,6 +67,7 @@ public void testExplicitPropertyMappings() .setMetadataCacheTtl(new Duration(1, SECONDS)) .setSchemaNamesCacheTtl(new Duration(2, SECONDS)) .setTableNamesCacheTtl(new Duration(3, SECONDS)) + .setStatisticsCacheTtl(new Duration(7, SECONDS)) .setCacheMissing(true) .setCacheMaximumSize(5000); @@ -96,24 +97,32 @@ public void testCacheConfigValidation() .setTableNamesCacheTtl(new Duration(3, SECONDS)) .setCacheMaximumSize(5000)); + assertValidates(new BaseJdbcConfig() + .setConnectionUrl("jdbc:h2:mem:config") + .setStatisticsCacheTtl(new Duration(7, SECONDS)) + .setCacheMaximumSize(5000)); + assertValidates(new BaseJdbcConfig() .setConnectionUrl("jdbc:h2:mem:config") .setMetadataCacheTtl(new Duration(1, SECONDS))); - assertFailsValidation(new BaseJdbcConfig() - .setCacheMaximumSize(5000), + assertFailsValidation( + new BaseJdbcConfig() + .setCacheMaximumSize(5000), "cacheMaximumSizeConsistent", - "metadata.cache-ttl must be set to a non-zero value when metadata.cache-maximum-size is set", + "metadata.cache-ttl or metadata.statistics.cache-ttl must be set to a non-zero value when metadata.cache-maximum-size is set", AssertTrue.class); - assertFailsValidation(new BaseJdbcConfig() - .setSchemaNamesCacheTtl(new Duration(1, SECONDS)), + assertFailsValidation( + new BaseJdbcConfig() + .setSchemaNamesCacheTtl(new Duration(1, SECONDS)), "schemaNamesCacheTtlConsistent", "metadata.schemas.cache-ttl must not be set when metadata.cache-ttl is not set", AssertTrue.class); - assertFailsValidation(new BaseJdbcConfig() - .setTableNamesCacheTtl(new Duration(1, SECONDS)), + assertFailsValidation( + new BaseJdbcConfig() + .setTableNamesCacheTtl(new Duration(1, SECONDS)), "tableNamesCacheTtlConsistent", "metadata.tables.cache-ttl must not be set when metadata.cache-ttl is not set", AssertTrue.class); diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestCachingJdbcClient.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestCachingJdbcClient.java index 75b628f241d2..691b058ed337 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestCachingJdbcClient.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestCachingJdbcClient.java @@ -13,10 +13,12 @@ */ package io.trino.plugin.jdbc; +import com.google.common.base.Ticker; import com.google.common.cache.CacheStats; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Futures; +import io.airlift.testing.TestingTicker; import io.airlift.units.Duration; import io.trino.plugin.base.session.SessionPropertiesProvider; import io.trino.plugin.jdbc.JdbcProcedureHandle.ProcedureQuery; @@ -61,17 +63,16 @@ import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.MoreCollectors.onlyElement; import static io.airlift.concurrent.Threads.daemonThreadsNamed; +import static io.airlift.units.Duration.ZERO; import static io.trino.spi.session.PropertyMetadata.stringProperty; import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden; import static io.trino.spi.type.IntegerType.INTEGER; import static io.trino.testing.TestingNames.randomNameSuffix; -import static io.trino.testing.assertions.Assert.assertEventually; import static java.lang.String.format; import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; import static java.util.concurrent.Executors.newCachedThreadPool; import static java.util.concurrent.TimeUnit.DAYS; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.function.Function.identity; import static org.assertj.core.api.Assertions.assertThat; @@ -81,8 +82,7 @@ @TestInstance(PER_METHOD) public class TestCachingJdbcClient { - private static final Duration FOREVER = Duration.succinctDuration(1, DAYS); - private static final Duration ZERO = Duration.succinctDuration(0, MILLISECONDS); + private static final Duration FOREVER = new Duration(1, DAYS); private static final ImmutableList> PROPERTY_METADATA = ImmutableList.of( stringProperty( @@ -102,7 +102,6 @@ public class TestCachingJdbcClient .build(); private TestingDatabase database; - private CachingJdbcClient cachingJdbcClient; private JdbcClient jdbcClient; private String schema; private ExecutorService executor; @@ -112,43 +111,11 @@ public void setUp() throws Exception { database = new TestingDatabase(); - cachingJdbcClient = createCachingJdbcClient(true, 10000); jdbcClient = database.getJdbcClient(); schema = jdbcClient.getSchemaNames(SESSION).iterator().next(); executor = newCachedThreadPool(daemonThreadsNamed("TestCachingJdbcClient-%s")); } - private CachingJdbcClient createCachingJdbcClient( - Duration cacheTtl, - boolean cacheMissing, - long cacheMaximumSize) - { - return createCachingJdbcClient(cacheTtl, cacheTtl, cacheTtl, cacheMissing, cacheMaximumSize); - } - - private CachingJdbcClient createCachingJdbcClient( - Duration cacheTtl, - Duration schemasCacheTtl, - Duration tablesCacheTtl, - boolean cacheMissing, - long cacheMaximumSize) - { - return new CachingJdbcClient( - database.getJdbcClient(), - SESSION_PROPERTIES_PROVIDERS, - new SingletonIdentityCacheMapping(), - cacheTtl, - schemasCacheTtl, - tablesCacheTtl, - cacheMissing, - cacheMaximumSize); - } - - private CachingJdbcClient createCachingJdbcClient(boolean cacheMissing, long cacheMaximumSize) - { - return createCachingJdbcClient(FOREVER, cacheMissing, cacheMaximumSize); - } - @AfterEach public void tearDown() throws Exception @@ -162,6 +129,7 @@ public void tearDown() @Test public void testSchemaNamesCached() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); String phantomSchema = "phantom_schema"; jdbcClient.createSchema(SESSION, phantomSchema); @@ -184,6 +152,7 @@ public void testSchemaNamesCached() @Test public void testTableNamesCached() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); createTable(phantomTable); @@ -206,6 +175,7 @@ public void testTableNamesCached() @Test public void testTableHandleCached() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); createTable(phantomTable); @@ -220,6 +190,9 @@ public void testTableHandleCached() public void testTableHandleOfQueryCached() throws Exception { + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .build(); SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); createTable(phantomTable); @@ -318,6 +291,7 @@ public void testTableHandleOfQueryCached() public void testProcedureHandleCached() throws Exception { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); createTable(phantomTable); @@ -347,24 +321,25 @@ public void testProcedureHandleCached() @Test public void testTableHandleInvalidatedOnColumnsModifications() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); JdbcTableHandle table = createTable(new SchemaTableName(schema, "a_table")); JdbcColumnHandle existingColumn = addColumn(table, "a_column"); // warm-up cache - assertTableHandlesByNameCacheIsInvalidated(table); + assertTableHandlesByNameCacheIsInvalidated(cachingJdbcClient, table); JdbcColumnHandle newColumn = addColumn(cachingJdbcClient, table, "new_column"); - assertTableHandlesByNameCacheIsInvalidated(table); + assertTableHandlesByNameCacheIsInvalidated(cachingJdbcClient, table); cachingJdbcClient.setColumnComment(SESSION, table, newColumn, Optional.empty()); - assertTableHandlesByNameCacheIsInvalidated(table); + assertTableHandlesByNameCacheIsInvalidated(cachingJdbcClient, table); cachingJdbcClient.renameColumn(SESSION, table, newColumn, "new_column_name"); - assertTableHandlesByNameCacheIsInvalidated(table); + assertTableHandlesByNameCacheIsInvalidated(cachingJdbcClient, table); cachingJdbcClient.dropColumn(SESSION, table, existingColumn); - assertTableHandlesByNameCacheIsInvalidated(table); + assertTableHandlesByNameCacheIsInvalidated(cachingJdbcClient, table); dropTable(table); } - private void assertTableHandlesByNameCacheIsInvalidated(JdbcTableHandle table) + private void assertTableHandlesByNameCacheIsInvalidated(CachingJdbcClient cachingJdbcClient, JdbcTableHandle table) { SchemaTableName tableName = table.asPlainTable().getSchemaTableName(); @@ -379,6 +354,9 @@ private void assertTableHandlesByNameCacheIsInvalidated(JdbcTableHandle table) @Test public void testEmptyTableHandleIsCachedWhenCacheMissingIsTrue() { + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .config(enableCache().setCacheMissing(true)) + .build(); SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); assertThat(cachingJdbcClient.getTableHandle(SESSION, phantomTable)).isEmpty(); @@ -391,7 +369,9 @@ public void testEmptyTableHandleIsCachedWhenCacheMissingIsTrue() @Test public void testEmptyTableHandleNotCachedWhenCacheMissingIsFalse() { - CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(false, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .config(enableCache().setCacheMissing(false)) + .build(); SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); assertThat(cachingJdbcClient.getTableHandle(SESSION, phantomTable)).isEmpty(); @@ -444,6 +424,7 @@ private void dropTable(SchemaTableName phantomTable) @Test public void testColumnsCached() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); JdbcTableHandle table = getAnyTable(schema); JdbcColumnHandle phantomColumn = addColumn(table); @@ -465,6 +446,7 @@ public void testColumnsCached() @Test public void testColumnsCachedPerSession() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); ConnectorSession firstSession = createSession("first"); ConnectorSession secondSession = createSession("second"); JdbcTableHandle table = getAnyTable(schema); @@ -505,6 +487,7 @@ public void testColumnsCachedPerSession() @Test public void testColumnsCacheInvalidationOnTableDrop() { + CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(); ConnectorSession firstSession = createSession("first"); ConnectorSession secondSession = createSession("second"); JdbcTableHandle firstTable = createTable(new SchemaTableName(schema, "first_table")); @@ -556,7 +539,9 @@ public void testColumnsCacheInvalidationOnTableDrop() @Test public void testColumnsNotCachedWhenCacheDisabled() { - CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(ZERO, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .config(new BaseJdbcConfig().setMetadataCacheTtl(ZERO)) + .build(); ConnectorSession firstSession = createSession("first"); ConnectorSession secondSession = createSession("second"); @@ -593,7 +578,9 @@ public void testColumnsNotCachedWhenCacheDisabled() @Test public void testGetTableStatistics() { - CachingJdbcClient cachingJdbcClient = cachingStatisticsAwareJdbcClient(FOREVER, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .build(); ConnectorSession session = createSession("first"); JdbcTableHandle first = createTable(new SchemaTableName(schema, "first")); @@ -641,7 +628,9 @@ public void testGetTableStatistics() @Test public void testCacheGetTableStatisticsWithQueryRelationHandle() { - CachingJdbcClient cachingJdbcClient = cachingStatisticsAwareJdbcClient(FOREVER, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .build(); ConnectorSession session = createSession("some test session name"); JdbcTableHandle first = createTable(new SchemaTableName(schema, "first")); @@ -688,7 +677,9 @@ public void testCacheGetTableStatisticsWithQueryRelationHandle() @Test public void testTruncateTable() { - CachingJdbcClient cachingJdbcClient = cachingStatisticsAwareJdbcClient(FOREVER, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .build(); ConnectorSession session = createSession("table"); JdbcTableHandle table = createTable(new SchemaTableName(schema, "table")); @@ -720,30 +711,12 @@ public void testTruncateTable() this.jdbcClient.dropTable(SESSION, table); } - private CachingJdbcClient cachingStatisticsAwareJdbcClient(Duration duration, boolean cacheMissing, long cacheMaximumSize) - { - JdbcClient jdbcClient = database.getJdbcClient(); - JdbcClient statsAwareJdbcClient = new ForwardingJdbcClient() - { - @Override - protected JdbcClient delegate() - { - return jdbcClient; - } - - @Override - public TableStatistics getTableStatistics(ConnectorSession session, JdbcTableHandle handle) - { - return NON_EMPTY_STATS; - } - }; - return new CachingJdbcClient(statsAwareJdbcClient, SESSION_PROPERTIES_PROVIDERS, new SingletonIdentityCacheMapping(), duration, cacheMissing, cacheMaximumSize); - } - @Test public void testCacheEmptyStatistics() { - CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(FOREVER, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .config(enableCache().setCacheMissing(true)) + .build(); ConnectorSession session = createSession("table"); JdbcTableHandle table = createTable(new SchemaTableName(schema, "table")); @@ -762,7 +735,9 @@ public void testCacheEmptyStatistics() @Test public void testGetTableStatisticsDoNotCacheEmptyWhenCachingMissingIsDisabled() { - CachingJdbcClient cachingJdbcClient = createCachingJdbcClient(FOREVER, false, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .config(enableCache().setCacheMissing(false)) + .build(); ConnectorSession session = createSession("table"); JdbcTableHandle table = createTable(new SchemaTableName(schema, "table")); @@ -781,15 +756,11 @@ public void testGetTableStatisticsDoNotCacheEmptyWhenCachingMissingIsDisabled() @Test public void testDifferentIdentityKeys() { - CachingJdbcClient cachingJdbcClient = new CachingJdbcClient( - database.getJdbcClient(), - SESSION_PROPERTIES_PROVIDERS, - new ExtraCredentialsBasedIdentityCacheMapping(new ExtraCredentialConfig() + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .identityCacheMapping(new ExtraCredentialsBasedIdentityCacheMapping(new ExtraCredentialConfig() .setUserCredentialName("user") - .setPasswordCredentialName("password")), - FOREVER, - true, - 10000); + .setPasswordCredentialName("password"))) + .build(); ConnectorSession alice = createUserSession("alice"); ConnectorSession bob = createUserSession("bob"); @@ -812,7 +783,9 @@ public void testDifferentIdentityKeys() @Test public void testFlushCache() { - CachingJdbcClient cachingJdbcClient = cachingStatisticsAwareJdbcClient(FOREVER, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .build(); ConnectorSession session = createSession("asession"); JdbcTableHandle first = createTable(new SchemaTableName(schema, "atable")); @@ -849,7 +822,9 @@ public void testFlushCache() @Timeout(60) public void testConcurrentSchemaCreateAndDrop() { - CachingJdbcClient cachingJdbcClient = cachingStatisticsAwareJdbcClient(FOREVER, true, 10000); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .build(); ConnectorSession session = createSession("asession"); List> futures = new ArrayList<>(); for (int i = 0; i < 5; i++) { @@ -875,8 +850,8 @@ public void testLoadFailureNotSharedWhenDisabled() AtomicBoolean first = new AtomicBoolean(true); CyclicBarrier barrier = new CyclicBarrier(2); - CachingJdbcClient cachingJdbcClient = new CachingJdbcClient( - new ForwardingJdbcClient() + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(new ForwardingJdbcClient() { private final JdbcClient delegate = database.getJdbcClient(); @@ -902,13 +877,10 @@ public Optional getTableHandle(ConnectorSession session, Schema } return super.getTableHandle(session, schemaTableName); } - }, - SESSION_PROPERTIES_PROVIDERS, - new SingletonIdentityCacheMapping(), + }) // ttl is 0, cache is disabled - new Duration(0, DAYS), - true, - 10); + .config(new BaseJdbcConfig().setMetadataCacheTtl(ZERO)) + .build(); SchemaTableName tableName = new SchemaTableName(schema, "test_load_failure_not_shared"); createTable(tableName); @@ -941,12 +913,16 @@ public Optional getTableHandle(ConnectorSession session, Schema @Test public void testSpecificSchemaAndTableCaches() { - CachingJdbcClient cachingJdbcClient = createCachingJdbcClient( - FOREVER, - Duration.succinctDuration(3, SECONDS), - Duration.succinctDuration(2, SECONDS), - false, // decreased ttl for schema and table names mostly makes sense with cacheMissing == false - 10000); + TestingTicker ticker = new TestingTicker(); + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .ticker(ticker) + .config(new BaseJdbcConfig() + .setMetadataCacheTtl(FOREVER) + .setSchemaNamesCacheTtl(new Duration(30, SECONDS)) + .setTableNamesCacheTtl(new Duration(20, SECONDS)) + // decreased ttl for schema and table names mostly makes sense with cacheMissing == false + .setCacheMissing(false)) + .build(); String secondSchema = schema + "_two"; SchemaTableName firstName = new SchemaTableName(schema, "first_table"); SchemaTableName secondName = new SchemaTableName(secondSchema, "second_table"); @@ -994,40 +970,38 @@ public void testSpecificSchemaAndTableCaches() }); // reloads table names, retains schema names and table handles - assertEventually(Duration.succinctDuration(10, SECONDS), () -> { - assertSchemaNamesCache(cachingJdbcClient).hits(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getSchemaNames(session)) - .contains(schema) - .doesNotContain(secondSchema); - }); - assertTableNamesCache(cachingJdbcClient).loads(1).misses(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getTableNames(session, Optional.empty())) - .contains(firstName, secondName); - }); - assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getTableHandle(session, firstName)).isNotEmpty(); - }); - assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getTableHandle(session, secondName)).isNotEmpty(); - }); + ticker.increment(25, SECONDS); + assertSchemaNamesCache(cachingJdbcClient).hits(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getSchemaNames(session)) + .contains(schema) + .doesNotContain(secondSchema); + }); + assertTableNamesCache(cachingJdbcClient).loads(1).misses(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getTableNames(session, Optional.empty())) + .contains(firstName, secondName); + }); + assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getTableHandle(session, firstName)).isNotEmpty(); + }); + assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getTableHandle(session, secondName)).isNotEmpty(); }); // reloads tables names and schema names, but retains table handles - assertEventually(Duration.succinctDuration(10, SECONDS), () -> { - assertSchemaNamesCache(cachingJdbcClient).loads(1).misses(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getSchemaNames(session)) - .contains(schema, secondSchema); - }); - assertTableNamesCache(cachingJdbcClient).loads(1).misses(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getTableNames(session, Optional.empty())) - .contains(firstName, secondName); - }); - assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getTableHandle(session, firstName)).isNotEmpty(); - }); - assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { - assertThat(cachingJdbcClient.getTableHandle(session, secondName)).isNotEmpty(); - }); + ticker.increment(35, SECONDS); + assertSchemaNamesCache(cachingJdbcClient).loads(1).misses(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getSchemaNames(session)) + .contains(schema, secondSchema); + }); + assertTableNamesCache(cachingJdbcClient).loads(1).misses(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getTableNames(session, Optional.empty())) + .contains(firstName, secondName); + }); + assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getTableHandle(session, firstName)).isNotEmpty(); + }); + assertTableHandleByNameCache(cachingJdbcClient).hits(1).afterRunning(() -> { + assertThat(cachingJdbcClient.getTableHandle(session, secondName)).isNotEmpty(); }); jdbcClient.dropTable(SESSION, first); @@ -1035,6 +1009,44 @@ public void testSpecificSchemaAndTableCaches() jdbcClient.dropSchema(SESSION, secondSchema, false); } + @Test + public void testCacheOnlyStatistics() + throws Exception + { + CachingJdbcClient cachingJdbcClient = cachingClientBuilder() + .delegate(jdbcClientWithTableStats()) + .config(new BaseJdbcConfig() + .setMetadataCacheTtl(ZERO) + .setStatisticsCacheTtl(FOREVER)) + .build(); + SchemaTableName phantomTable = new SchemaTableName(schema, "phantom_table"); + + createTable(phantomTable); + + JdbcTableHandle firstHandle = assertTableHandleByNameCache(cachingJdbcClient) + .misses(2) + .loads(1) + .calling(() -> cachingJdbcClient.getTableHandle(SESSION, phantomTable)).orElseThrow(); + + assertStatisticsCacheStats(cachingJdbcClient) + .misses(1) + .loads(1) + .calling(() -> cachingJdbcClient.getTableStatistics(SESSION, firstHandle)); + + // Handle is afresh, not cached + JdbcTableHandle secondHandle = assertTableHandleByNameCache(cachingJdbcClient) + .misses(2) + .loads(1) + .calling(() -> cachingJdbcClient.getTableHandle(SESSION, phantomTable)).orElseThrow(); + + // Stats come from the cache + assertStatisticsCacheStats(cachingJdbcClient) + .hits(1) + .calling(() -> cachingJdbcClient.getTableStatistics(SESSION, secondHandle)); + + dropTable(phantomTable); + } + private JdbcTableHandle getAnyTable(String schema) { SchemaTableName tableName = jdbcClient.getTableNames(SESSION, Optional.of(schema)) @@ -1088,6 +1100,98 @@ public void testEverythingImplemented() assertAllMethodsOverridden(JdbcClient.class, CachingJdbcClient.class); } + private CachingJdbcClient createCachingJdbcClient() + { + return cachingClientBuilder().build(); + } + + private CachingJdbcClientBuilder cachingClientBuilder() + { + return new CachingJdbcClientBuilder() + .ticker(Ticker.systemTicker()) + .delegate(database.getJdbcClient()) + .sessionPropertiesProviders(SESSION_PROPERTIES_PROVIDERS) + .identityCacheMapping(new SingletonIdentityCacheMapping()) + .config(enableCache()); + } + + private static class CachingJdbcClientBuilder + { + private Ticker ticker; + private JdbcClient delegate; + private Set sessionPropertiesProviders; + private IdentityCacheMapping identityCacheMapping; + private BaseJdbcConfig config; + + public CachingJdbcClientBuilder ticker(Ticker ticker) + { + this.ticker = ticker; + return this; + } + + public CachingJdbcClientBuilder delegate(JdbcClient delegate) + { + this.delegate = delegate; + return this; + } + + public CachingJdbcClientBuilder sessionPropertiesProviders(Set sessionPropertiesProviders) + { + this.sessionPropertiesProviders = sessionPropertiesProviders; + return this; + } + + public CachingJdbcClientBuilder identityCacheMapping(IdentityCacheMapping identityCacheMapping) + { + this.identityCacheMapping = identityCacheMapping; + return this; + } + + public CachingJdbcClientBuilder config(BaseJdbcConfig config) + { + this.config = config; + return this; + } + + CachingJdbcClient build() + { + return new CachingJdbcClient( + ticker, + delegate, + sessionPropertiesProviders, + identityCacheMapping, + config.getMetadataCacheTtl(), + config.getSchemaNamesCacheTtl(), + config.getTableNamesCacheTtl(), + config.getStatisticsCacheTtl(), + config.isCacheMissing(), + config.getCacheMaximumSize()); + } + } + + private JdbcClient jdbcClientWithTableStats() + { + return new ForwardingJdbcClient() + { + @Override + protected JdbcClient delegate() + { + return database.getJdbcClient(); + } + + @Override + public TableStatistics getTableStatistics(ConnectorSession session, JdbcTableHandle handle) + { + return NON_EMPTY_STATS; + } + }; + } + + private static BaseJdbcConfig enableCache() + { + return new BaseJdbcConfig().setMetadataCacheTtl(FOREVER); + } + private static SingleJdbcCacheStatsAssertions assertSchemaNamesCache(CachingJdbcClient client) { return assertCacheStats(client, CachingJdbcCache.SCHEMA_NAMES_CACHE);