diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/DecoratedHiveMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/DecoratedHiveMetastoreModule.java new file mode 100644 index 000000000000..1ee2ac1284b6 --- /dev/null +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/DecoratedHiveMetastoreModule.java @@ -0,0 +1,83 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.hive.metastore; + +import com.google.inject.Binder; +import com.google.inject.Provides; +import com.google.inject.Scopes; +import com.google.inject.Singleton; +import io.airlift.configuration.AbstractConfigurationAwareModule; +import io.trino.plugin.hive.metastore.cache.CachingHiveMetastore; +import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreConfig; +import io.trino.plugin.hive.metastore.cache.SharedHiveMetastoreCache; +import io.trino.plugin.hive.metastore.procedure.FlushHiveMetastoreCacheProcedure; +import io.trino.plugin.hive.metastore.recording.RecordingHiveMetastoreDecoratorModule; +import io.trino.spi.procedure.Procedure; + +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.inject.multibindings.Multibinder.newSetBinder; +import static io.airlift.configuration.ConfigBinder.configBinder; +import static org.weakref.jmx.guice.ExportBinder.newExporter; + +public class DecoratedHiveMetastoreModule + extends AbstractConfigurationAwareModule +{ + @Override + protected void setup(Binder binder) + { + newSetBinder(binder, HiveMetastoreDecorator.class); + install(new RecordingHiveMetastoreDecoratorModule()); + + configBinder(binder).bindConfig(CachingHiveMetastoreConfig.class); + binder.bind(SharedHiveMetastoreCache.class).in(Scopes.SINGLETON); + newExporter(binder).export(HiveMetastore.class) + .as(generator -> generator.generatedNameOf(CachingHiveMetastore.class)); + + newSetBinder(binder, Procedure.class).addBinding().toProvider(FlushHiveMetastoreCacheProcedure.class).in(Scopes.SINGLETON); + } + + @Provides + @Singleton + public static HiveMetastore createHiveMetastore( + @RawHiveMetastore HiveMetastore metastore, + Set decorators, + SharedHiveMetastoreCache sharedHiveMetastoreCache) + { + // wrap the raw metastore with decorators like the RecordingHiveMetastore + List sortedDecorators = decorators.stream() + .sorted(Comparator.comparing(HiveMetastoreDecorator::getPriority)) + .collect(toImmutableList()); + for (HiveMetastoreDecorator decorator : sortedDecorators) { + metastore = decorator.decorate(metastore); + } + + // finally, if the shared metastore cache is enabled wrapper with a global cache + return sharedHiveMetastoreCache.createSharedHiveMetastoreCache(metastore); + } + + @Provides + @Singleton + public static Optional createHiveMetastore(HiveMetastore metastore) + { + if (metastore instanceof CachingHiveMetastore) { + return Optional.of((CachingHiveMetastore) metastore); + } + return Optional.empty(); + } +} diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreDecorator.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreDecorator.java index 21b90d05bceb..f6a6c41381f1 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreDecorator.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreDecorator.java @@ -16,5 +16,10 @@ public interface HiveMetastoreDecorator { + int PRIORITY_INTIAL = 0; + int PRIORITY_RECORDING = 100; + + int getPriority(); + HiveMetastore decorate(HiveMetastore hiveMetastore); } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreModule.java index c37d7af45f6e..6583f03fd23d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/HiveMetastoreModule.java @@ -17,8 +17,6 @@ import com.google.inject.Module; import io.airlift.configuration.AbstractConfigurationAwareModule; import io.trino.plugin.hive.metastore.alluxio.AlluxioMetastoreModule; -import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreModule; -import io.trino.plugin.hive.metastore.cache.ForCachingHiveMetastore; import io.trino.plugin.hive.metastore.file.FileMetastoreModule; import io.trino.plugin.hive.metastore.glue.GlueMetastoreModule; import io.trino.plugin.hive.metastore.thrift.ThriftMetastoreModule; @@ -41,8 +39,7 @@ public HiveMetastoreModule(Optional metastore) protected void setup(Binder binder) { if (metastore.isPresent()) { - binder.bind(HiveMetastore.class).annotatedWith(ForCachingHiveMetastore.class).toInstance(metastore.get()); - install(new CachingHiveMetastoreModule()); + binder.bind(HiveMetastore.class).annotatedWith(RawHiveMetastore.class).toInstance(metastore.get()); } else { bindMetastoreModule("thrift", new ThriftMetastoreModule()); @@ -50,6 +47,8 @@ protected void setup(Binder binder) bindMetastoreModule("glue", new GlueMetastoreModule()); bindMetastoreModule("alluxio", new AlluxioMetastoreModule()); } + + install(new DecoratedHiveMetastoreModule()); } private void bindMetastoreModule(String name, Module module) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ForRecordingHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/RawHiveMetastore.java similarity index 92% rename from plugin/trino-hive/src/main/java/io/trino/plugin/hive/ForRecordingHiveMetastore.java rename to plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/RawHiveMetastore.java index 304c26470623..6ba123e442b7 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ForRecordingHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/RawHiveMetastore.java @@ -11,7 +11,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.trino.plugin.hive; +package io.trino.plugin.hive.metastore; import javax.inject.Qualifier; @@ -26,6 +26,6 @@ @Retention(RUNTIME) @Target({FIELD, PARAMETER, METHOD}) @Qualifier -public @interface ForRecordingHiveMetastore +public @interface RawHiveMetastore { } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/alluxio/AlluxioMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/alluxio/AlluxioMetastoreModule.java index c4ea691b82f0..a9506548cd8f 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/alluxio/AlluxioMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/alluxio/AlluxioMetastoreModule.java @@ -25,9 +25,9 @@ import com.google.inject.Scopes; import io.airlift.configuration.AbstractConfigurationAwareModule; import io.trino.plugin.hive.metastore.HiveMetastore; +import io.trino.plugin.hive.metastore.RawHiveMetastore; import static io.airlift.configuration.ConfigBinder.configBinder; -import static org.weakref.jmx.guice.ExportBinder.newExporter; /** * Module for an Alluxio metastore implementation of the {@link HiveMetastore} interface. @@ -40,8 +40,7 @@ protected void setup(Binder binder) { configBinder(binder).bindConfig(AlluxioHiveMetastoreConfig.class); - binder.bind(HiveMetastore.class).to(AlluxioHiveMetastore.class).in(Scopes.SINGLETON); - newExporter(binder).export(HiveMetastore.class).as(generator -> generator.generatedNameOf(AlluxioHiveMetastore.class)); + binder.bind(HiveMetastore.class).annotatedWith(RawHiveMetastore.class).to(AlluxioHiveMetastore.class).in(Scopes.SINGLETON); } @Provides diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java index 515454701c26..50313456c12f 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastore.java @@ -73,7 +73,6 @@ import static com.google.common.base.Functions.identity; import static com.google.common.base.MoreObjects.toStringHelper; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Throwables.throwIfInstanceOf; import static com.google.common.base.Throwables.throwIfUnchecked; import static com.google.common.cache.CacheLoader.asyncReloading; @@ -88,8 +87,6 @@ import static io.trino.plugin.hive.metastore.HiveTableName.hiveTableName; import static io.trino.plugin.hive.metastore.MetastoreUtil.makePartitionName; import static io.trino.plugin.hive.metastore.PartitionFilter.partitionFilter; -import static io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreConfig.isCacheEnabled; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; import static org.apache.hadoop.hive.common.FileUtils.makePartName; @@ -123,21 +120,8 @@ public enum StatsRecording private final LoadingCache> grantedPrincipalsCache; private final LoadingCache> configValuesCache; - public static CachingHiveMetastore cachingHiveMetastore(HiveMetastore delegate, Executor executor, CachingHiveMetastoreConfig config) - { - return cachingHiveMetastore( - delegate, - executor, - config.getMetastoreCacheTtl(), - config.getMetastoreRefreshInterval(), - config.getMetastoreCacheMaximumSize()); - } - public static CachingHiveMetastore cachingHiveMetastore(HiveMetastore delegate, Executor executor, Duration cacheTtl, Optional refreshInterval, long maximumSize) { - checkState( - isCacheEnabled(cacheTtl, maximumSize), - format("Invalid cache parameters (cacheTtl: %s, maxSize: %s)", cacheTtl, maximumSize)); return new CachingHiveMetastore( delegate, OptionalLong.of(cacheTtl.toMillis()), diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreConfig.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreConfig.java index b1e860eb93d6..7e4f98d55fb6 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreConfig.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreConfig.java @@ -81,16 +81,4 @@ public CachingHiveMetastoreConfig setMaxMetastoreRefreshThreads(int maxMetastore this.maxMetastoreRefreshThreads = maxMetastoreRefreshThreads; return this; } - - public boolean isCacheEnabled() - { - return isCacheEnabled( - getMetastoreCacheTtl(), - getMetastoreCacheMaximumSize()); - } - - public static boolean isCacheEnabled(Duration metastoreCacheTtl, long metastoreCacheMaximumSize) - { - return metastoreCacheTtl.toMillis() != 0 && metastoreCacheMaximumSize != 0; - } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreModule.java deleted file mode 100644 index f4f82cfc7cb0..000000000000 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/CachingHiveMetastoreModule.java +++ /dev/null @@ -1,112 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.trino.plugin.hive.metastore.cache; - -import com.google.inject.Binder; -import com.google.inject.Module; -import com.google.inject.Provides; -import com.google.inject.Scopes; -import io.trino.plugin.base.CatalogName; -import io.trino.plugin.hive.metastore.HiveMetastore; -import io.trino.plugin.hive.metastore.HiveMetastoreDecorator; -import io.trino.plugin.hive.metastore.procedure.FlushHiveMetastoreCacheProcedure; -import io.trino.spi.NodeManager; -import io.trino.spi.procedure.Procedure; - -import javax.inject.Qualifier; -import javax.inject.Singleton; - -import java.lang.annotation.Retention; -import java.lang.annotation.Target; -import java.util.Optional; - -import static com.google.inject.multibindings.Multibinder.newSetBinder; -import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder; -import static io.airlift.concurrent.Threads.daemonThreadsNamed; -import static io.airlift.configuration.ConfigBinder.configBinder; -import static io.trino.plugin.hive.metastore.cache.CachingHiveMetastore.cachingHiveMetastore; -import static java.lang.annotation.ElementType.FIELD; -import static java.lang.annotation.ElementType.METHOD; -import static java.lang.annotation.ElementType.PARAMETER; -import static java.lang.annotation.RetentionPolicy.RUNTIME; -import static java.util.concurrent.Executors.newCachedThreadPool; -import static org.weakref.jmx.guice.ExportBinder.newExporter; - -public class CachingHiveMetastoreModule - implements Module -{ - @Override - public void configure(Binder binder) - { - configBinder(binder).bindConfig(CachingHiveMetastoreConfig.class); - newOptionalBinder(binder, HiveMetastoreDecorator.class); - newExporter(binder).export(HiveMetastore.class) - .as(generator -> generator.generatedNameOf(CachingHiveMetastore.class)); - newSetBinder(binder, Procedure.class).addBinding().toProvider(FlushHiveMetastoreCacheProcedure.class).in(Scopes.SINGLETON); - } - - @Provides - @Singleton - @DecoratedForCachingHiveMetastore - public HiveMetastore createDecoratedHiveMetastore( - @ForCachingHiveMetastore HiveMetastore delegate, - Optional hiveMetastoreDecorator) - { - return hiveMetastoreDecorator - .map(decorator -> decorator.decorate(delegate)) - .orElse(delegate); - } - - @Provides - @Singleton - public Optional createCachingHiveMetastore( - NodeManager nodeManager, - @DecoratedForCachingHiveMetastore HiveMetastore delegate, - CachingHiveMetastoreConfig config, - CatalogName catalogName) - { - if (!nodeManager.getCurrentNode().isCoordinator() || !config.isCacheEnabled()) { - // Disable caching on workers, because there currently is no way to invalidate such a cache. - // Note: while we could skip CachingHiveMetastoreModule altogether on workers, we retain it so that catalog - // configuration can remain identical for all nodes, making cluster configuration easier. - return Optional.empty(); - } - - return Optional.of(cachingHiveMetastore( - delegate, - // Loading of cache entry in CachingHiveMetastore might trigger loading of another cache entry for different object type - // In case there are no empty executor slots, such operation would deadlock. Therefore, a reentrant executor needs to be - // used. - new ReentrantBoundedExecutor( - newCachedThreadPool(daemonThreadsNamed("hive-metastore-" + catalogName + "-%s")), - config.getMaxMetastoreRefreshThreads()), - config)); - } - - @Provides - @Singleton - public HiveMetastore createHiveMetastore( - @DecoratedForCachingHiveMetastore HiveMetastore delegate, - Optional cachingMetastore) - { - return cachingMetastore.map(metastore -> (HiveMetastore) metastore).orElse(delegate); - } - - @Retention(RUNTIME) - @Target({FIELD, PARAMETER, METHOD}) - @Qualifier - public @interface DecoratedForCachingHiveMetastore - { - } -} diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/ForCachingHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/ForCachingHiveMetastore.java deleted file mode 100644 index 25af522907d7..000000000000 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/ForCachingHiveMetastore.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.trino.plugin.hive.metastore.cache; - -import javax.inject.Qualifier; - -import java.lang.annotation.Retention; -import java.lang.annotation.Target; - -import static java.lang.annotation.ElementType.FIELD; -import static java.lang.annotation.ElementType.METHOD; -import static java.lang.annotation.ElementType.PARAMETER; -import static java.lang.annotation.RetentionPolicy.RUNTIME; - -@Retention(RUNTIME) -@Target({FIELD, PARAMETER, METHOD}) -@Qualifier -public @interface ForCachingHiveMetastore -{ -} diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/SharedHiveMetastoreCache.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/SharedHiveMetastoreCache.java new file mode 100644 index 000000000000..5bb5520e78e0 --- /dev/null +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/cache/SharedHiveMetastoreCache.java @@ -0,0 +1,101 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.hive.metastore.cache; + +import io.airlift.units.Duration; +import io.trino.plugin.base.CatalogName; +import io.trino.plugin.hive.metastore.HiveMetastore; +import io.trino.spi.NodeManager; + +import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; +import javax.inject.Inject; + +import java.util.Optional; +import java.util.concurrent.ExecutorService; + +import static io.airlift.concurrent.Threads.daemonThreadsNamed; +import static io.trino.plugin.hive.metastore.cache.CachingHiveMetastore.cachingHiveMetastore; +import static java.util.Objects.requireNonNull; +import static java.util.concurrent.Executors.newCachedThreadPool; + +public class SharedHiveMetastoreCache +{ + private final boolean enabled; + private final CatalogName catalogName; + private final Duration metastoreCacheTtl; + private final Optional metastoreRefreshInterval; + private final long metastoreCacheMaximumSize; + private final int maxMetastoreRefreshThreads; + + private ExecutorService executorService; + + @Inject + public SharedHiveMetastoreCache( + CatalogName catalogName, + NodeManager nodeManager, + CachingHiveMetastoreConfig config) + { + requireNonNull(nodeManager, "nodeManager is null"); + requireNonNull(config, "config is null"); + requireNonNull(catalogName, "catalogName is null"); + + this.catalogName = catalogName; + maxMetastoreRefreshThreads = config.getMaxMetastoreRefreshThreads(); + metastoreCacheTtl = config.getMetastoreCacheTtl(); + metastoreRefreshInterval = config.getMetastoreRefreshInterval(); + metastoreCacheMaximumSize = config.getMetastoreCacheMaximumSize(); + + // Disable caching on workers, because there currently is no way to invalidate such a cache. + // Note: while we could skip CachingHiveMetastoreModule altogether on workers, we retain it so that catalog + // configuration can remain identical for all nodes, making cluster configuration easier. + enabled = nodeManager.getCurrentNode().isCoordinator() && + metastoreCacheTtl.toMillis() > 0 && + metastoreCacheMaximumSize > 0; + } + + @PostConstruct + public void start() + { + if (enabled) { + executorService = newCachedThreadPool(daemonThreadsNamed("hive-metastore-" + catalogName + "-%s")); + } + } + + @PreDestroy + public void stop() + { + if (executorService != null) { + executorService.shutdownNow(); + executorService = null; + } + } + + public HiveMetastore createSharedHiveMetastoreCache(HiveMetastore hiveMetastore) + { + if (!enabled) { + return hiveMetastore; + } + + return cachingHiveMetastore( + hiveMetastore, + // Loading of cache entry in CachingHiveMetastore might trigger loading of another cache entry for different object type + // In case there are no empty executor slots, such operation would deadlock. Therefore, a reentrant executor needs to be + // used. + new ReentrantBoundedExecutor(executorService, maxMetastoreRefreshThreads), + metastoreCacheTtl, + metastoreRefreshInterval, + metastoreCacheMaximumSize); + } +} diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileMetastoreModule.java index c9370d66e810..65ed881d435d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileMetastoreModule.java @@ -17,8 +17,7 @@ import com.google.inject.Module; import com.google.inject.Scopes; import io.trino.plugin.hive.metastore.HiveMetastore; -import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreModule; -import io.trino.plugin.hive.metastore.cache.ForCachingHiveMetastore; +import io.trino.plugin.hive.metastore.RawHiveMetastore; import static io.airlift.configuration.ConfigBinder.configBinder; @@ -29,7 +28,6 @@ public class FileMetastoreModule public void configure(Binder binder) { configBinder(binder).bindConfig(FileHiveMetastoreConfig.class); - binder.bind(HiveMetastore.class).annotatedWith(ForCachingHiveMetastore.class).to(FileHiveMetastore.class).in(Scopes.SINGLETON); - binder.install(new CachingHiveMetastoreModule()); + binder.bind(HiveMetastore.class).annotatedWith(RawHiveMetastore.class).to(FileHiveMetastore.class).in(Scopes.SINGLETON); } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueMetastoreModule.java index 864ef7e69a8f..1f3c4ef1a9fb 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueMetastoreModule.java @@ -25,11 +25,9 @@ import io.airlift.concurrent.BoundedExecutor; import io.airlift.configuration.AbstractConfigurationAwareModule; import io.trino.plugin.base.CatalogName; -import io.trino.plugin.hive.ForRecordingHiveMetastore; import io.trino.plugin.hive.HiveConfig; import io.trino.plugin.hive.metastore.HiveMetastore; -import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreModule; -import io.trino.plugin.hive.metastore.recording.RecordingHiveMetastoreModule; +import io.trino.plugin.hive.metastore.RawHiveMetastore; import java.util.concurrent.Executor; import java.util.function.Predicate; @@ -56,7 +54,7 @@ protected void setup(Binder binder) .setDefault().toProvider(DefaultGlueMetastoreTableFilterProvider.class).in(Scopes.SINGLETON); binder.bind(HiveMetastore.class) - .annotatedWith(ForRecordingHiveMetastore.class) + .annotatedWith(RawHiveMetastore.class) .to(GlueHiveMetastore.class) .in(Scopes.SINGLETON); @@ -68,9 +66,6 @@ protected void setup(Binder binder) HiveConfig::isTableStatisticsEnabled, getGlueStatisticsModule(DefaultGlueColumnStatisticsProviderFactory.class), getGlueStatisticsModule(DisabledGlueColumnStatisticsProviderFactory.class))); - - install(new RecordingHiveMetastoreModule()); - install(new CachingHiveMetastoreModule()); } private Module getGlueStatisticsModule(Class statisticsPrividerFactoryClass) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastore.java index 43e53bba23ed..1a1915e45bed 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastore.java @@ -13,7 +13,6 @@ */ package io.trino.plugin.hive.metastore.recording; -import io.trino.plugin.hive.ForRecordingHiveMetastore; import io.trino.plugin.hive.HiveType; import io.trino.plugin.hive.PartitionStatistics; import io.trino.plugin.hive.acid.AcidTransaction; @@ -34,8 +33,6 @@ import io.trino.spi.statistics.ColumnStatisticType; import io.trino.spi.type.Type; -import javax.inject.Inject; - import java.util.List; import java.util.Map; import java.util.Optional; @@ -54,8 +51,7 @@ public class RecordingHiveMetastore private final HiveMetastore delegate; private final HiveMetastoreRecording recording; - @Inject - public RecordingHiveMetastore(@ForRecordingHiveMetastore HiveMetastore delegate, HiveMetastoreRecording recording) + public RecordingHiveMetastore(HiveMetastore delegate, HiveMetastoreRecording recording) { this.delegate = requireNonNull(delegate, "delegate is null"); this.recording = requireNonNull(recording, "recording is null"); diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreDecorator.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreDecorator.java new file mode 100644 index 000000000000..1a123d4ffeff --- /dev/null +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreDecorator.java @@ -0,0 +1,45 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.hive.metastore.recording; + +import io.trino.plugin.hive.metastore.HiveMetastore; +import io.trino.plugin.hive.metastore.HiveMetastoreDecorator; + +import javax.inject.Inject; + +import static java.util.Objects.requireNonNull; + +public class RecordingHiveMetastoreDecorator + implements HiveMetastoreDecorator +{ + private final HiveMetastoreRecording recording; + + @Inject + public RecordingHiveMetastoreDecorator(HiveMetastoreRecording recording) + { + this.recording = requireNonNull(recording, "recording is null"); + } + + @Override + public int getPriority() + { + return PRIORITY_RECORDING; + } + + @Override + public HiveMetastore decorate(HiveMetastore hiveMetastore) + { + return new RecordingHiveMetastore(hiveMetastore, recording); + } +} diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreDecoratorModule.java similarity index 64% rename from plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreModule.java rename to plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreDecoratorModule.java index 8b918623dcd5..e140e836393d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/recording/RecordingHiveMetastoreDecoratorModule.java @@ -14,14 +14,10 @@ package io.trino.plugin.hive.metastore.recording; import com.google.inject.Binder; -import com.google.inject.Key; -import com.google.inject.Module; import com.google.inject.Scopes; import io.airlift.configuration.AbstractConfigurationAwareModule; -import io.trino.plugin.hive.ForRecordingHiveMetastore; import io.trino.plugin.hive.RecordingMetastoreConfig; -import io.trino.plugin.hive.metastore.HiveMetastore; -import io.trino.plugin.hive.metastore.cache.ForCachingHiveMetastore; +import io.trino.plugin.hive.metastore.HiveMetastoreDecorator; import io.trino.plugin.hive.util.BlockJsonSerde; import io.trino.plugin.hive.util.HiveBlockEncodingSerde; import io.trino.spi.block.Block; @@ -32,31 +28,14 @@ import static io.airlift.json.JsonCodecBinder.jsonCodecBinder; import static org.weakref.jmx.guice.ExportBinder.newExporter; -public class RecordingHiveMetastoreModule +public class RecordingHiveMetastoreDecoratorModule extends AbstractConfigurationAwareModule { @Override protected void setup(Binder binder) { if (buildConfigObject(RecordingMetastoreConfig.class).getRecordingPath() != null) { - install(new RecordingModule()); - } - else { - install(new NoRecordingModule()); - } - } - - public static class RecordingModule - implements Module - { - @Override - public void configure(Binder binder) - { - binder.bind(HiveMetastore.class) - .annotatedWith(ForCachingHiveMetastore.class) - .to(RecordingHiveMetastore.class) - .in(Scopes.SINGLETON); - binder.bind(RecordingHiveMetastore.class).in(Scopes.SINGLETON); + newSetBinder(binder, HiveMetastoreDecorator.class).addBinding().to(RecordingHiveMetastoreDecorator.class).in(Scopes.SINGLETON); binder.bind(HiveBlockEncodingSerde.class).in(Scopes.SINGLETON); binder.bind(HiveMetastoreRecording.class).in(Scopes.SINGLETON); @@ -70,17 +49,4 @@ public void configure(Binder binder) newSetBinder(binder, Procedure.class).addBinding().toProvider(WriteHiveMetastoreRecordingProcedure.class).in(Scopes.SINGLETON); } } - - public static class NoRecordingModule - implements Module - { - @Override - public void configure(Binder binder) - { - binder.bind(HiveMetastore.class) - .annotatedWith(ForCachingHiveMetastore.class) - .to(Key.get(HiveMetastore.class, ForRecordingHiveMetastore.class)) - .in(Scopes.SINGLETON); - } - } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreModule.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreModule.java index 765c8026d9c5..c6a625599c0e 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreModule.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreModule.java @@ -17,10 +17,8 @@ import com.google.inject.Scopes; import com.google.inject.multibindings.OptionalBinder; import io.airlift.configuration.AbstractConfigurationAwareModule; -import io.trino.plugin.hive.ForRecordingHiveMetastore; import io.trino.plugin.hive.metastore.HiveMetastore; -import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreModule; -import io.trino.plugin.hive.metastore.recording.RecordingHiveMetastoreModule; +import io.trino.plugin.hive.metastore.RawHiveMetastore; import static io.airlift.configuration.ConfigBinder.configBinder; import static org.weakref.jmx.guice.ExportBinder.newExporter; @@ -42,13 +40,10 @@ protected void setup(Binder binder) .as(generator -> generator.generatedNameOf(ThriftHiveMetastore.class)); binder.bind(HiveMetastore.class) - .annotatedWith(ForRecordingHiveMetastore.class) + .annotatedWith(RawHiveMetastore.class) .to(BridgingHiveMetastore.class) .in(Scopes.SINGLETON); - install(new RecordingHiveMetastoreModule()); - install(new CachingHiveMetastoreModule()); - install(new ThriftMetastoreAuthenticationModule()); } } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/cache/TestCachingHiveMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/cache/TestCachingHiveMetastore.java index 6a62c3d1d08b..9ba23646e0b4 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/cache/TestCachingHiveMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/cache/TestCachingHiveMetastore.java @@ -671,7 +671,9 @@ private CachingHiveMetastore createMetastoreWithDirectExecutor(CachingHiveMetast return (CachingHiveMetastore) cachingHiveMetastore( new BridgingHiveMetastore(createThriftHiveMetastore()), directExecutor(), - config); + config.getMetastoreCacheTtl(), + config.getMetastoreRefreshInterval(), + config.getMetastoreCacheMaximumSize()); } private static class MockMetastoreLocator diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergCatalogModule.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergCatalogModule.java index 85c7f093a8a6..bedf9530e6af 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergCatalogModule.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergCatalogModule.java @@ -17,10 +17,10 @@ import com.google.inject.Module; import com.google.inject.Scopes; import io.airlift.configuration.AbstractConfigurationAwareModule; +import io.trino.plugin.hive.metastore.DecoratedHiveMetastoreModule; import io.trino.plugin.hive.metastore.HiveMetastore; +import io.trino.plugin.hive.metastore.RawHiveMetastore; import io.trino.plugin.hive.metastore.cache.CachingHiveMetastore; -import io.trino.plugin.hive.metastore.cache.CachingHiveMetastoreModule; -import io.trino.plugin.hive.metastore.cache.ForCachingHiveMetastore; import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider; import io.trino.plugin.iceberg.catalog.file.FileMetastoreTableOperationsProvider; import io.trino.plugin.iceberg.catalog.file.IcebergFileMetastoreCatalogModule; @@ -49,8 +49,7 @@ public IcebergCatalogModule(Optional metastore) protected void setup(Binder binder) { if (metastore.isPresent()) { - binder.bind(HiveMetastore.class).annotatedWith(ForCachingHiveMetastore.class).toInstance(metastore.get()); - install(new CachingHiveMetastoreModule()); + binder.bind(HiveMetastore.class).annotatedWith(RawHiveMetastore.class).toInstance(metastore.get()); binder.bind(IcebergTableOperationsProvider.class).to(FileMetastoreTableOperationsProvider.class).in(Scopes.SINGLETON); } else { @@ -60,6 +59,7 @@ protected void setup(Binder binder) } binder.bind(MetastoreValidator.class).asEagerSingleton(); + install(new DecoratedHiveMetastoreModule()); } public static class MetastoreValidator diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPlugin.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPlugin.java index 860f72712838..edd50f9b010d 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPlugin.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergPlugin.java @@ -99,17 +99,6 @@ public void testRecordingMetastore() "hive.metastore-recording-path", "/tmp"), new TestingConnectorContext()) .shutdown(); - - // recording with glue - assertThatThrownBy(() -> factory.create( - "test", - Map.of( - "iceberg.catalog.type", "glue", - "hive.metastore.glue.region", "us-east-2", - "hive.metastore-recording-path", "/tmp"), - new TestingConnectorContext())) - .hasMessageContaining("Configuration property 'hive.metastore-recording-path' was not used") - .hasMessageContaining("Configuration property 'hive.metastore.glue.region' was not used"); } @Test