Simplify HiveMetastore construction and bindings

Replace chained Guild binding annotations ForRecordingHiveMetastore and
ForCachingHiveMetastore with generic module to apply decorators and
shared caching.
Convert RecordingHiveMetastore to a decorator.
Add SharedHiveMetastoreCache and manually apply during Metastore
creation.

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<HiveMetastoreDecorator> decorators,
+            SharedHiveMetastoreCache sharedHiveMetastoreCache)
+    {
+        // wrap the raw metastore with decorators like the RecordingHiveMetastore
+        List<HiveMetastoreDecorator> 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<CachingHiveMetastore> createHiveMetastore(HiveMetastore metastore)
+    {
+        if (metastore instanceof CachingHiveMetastore) {
+            return Optional.of((CachingHiveMetastore) metastore);
+        }
+        return Optional.empty();
+    }
+}

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);
 }

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,15 +39,16 @@ public HiveMetastoreModule(Optional<HiveMetastore> 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());
             bindMetastoreModule("file", new FileMetastoreModule());
             bindMetastoreModule("glue", new GlueMetastoreModule());
             bindMetastoreModule("alluxio", new AlluxioMetastoreModule());
         }
+
+        install(new DecoratedHiveMetastoreModule());
     }
 
     private void bindMetastoreModule(String name, Module module)

plugin/trino-hive/src/main/java/io/trino/plugin/hive/ForRecordingHiveMetastore.java → 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
 {
 }

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

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<String, Set<RoleGrant>> grantedPrincipalsCache;
     private final LoadingCache<String, Optional<String>> 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<Duration> 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()),

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;
-    }
 }