Support hive.metastore.glue.skip-archive in v1 Glue

trinodb · Oct 24, 2024 · f7a47e3 · f7a47e3
1 parent b893c3d
commit f7a47e3
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 6 deletions.
diff --git a/...no-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueHiveMetastoreConfig.java b/...no-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueHiveMetastoreConfig.java
@@ -48,6 +48,7 @@ public class GlueHiveMetastoreConfig
     private int readStatisticsThreads = 5;
     private int writeStatisticsThreads = 20;
     private boolean assumeCanonicalPartitionKeys;
+    private boolean skipArchive;
 
     public Optional<String> getGlueRegion()
     {
@@ -317,6 +318,19 @@ public GlueHiveMetastoreConfig setAssumeCanonicalPartitionKeys(boolean assumeCan
         return this;
     }
 
+    public boolean isSkipArchive()
+    {
+        return skipArchive;
+    }
+
+    @Config("hive.metastore.glue.skip-archive")
+    @ConfigDescription("Skip archiving an old table version when creating a new version in a commit")
+    public GlueHiveMetastoreConfig setSkipArchive(boolean skipArchive)
+    {
+        this.skipArchive = skipArchive;
+        return this;
+    }
+
     @PostConstruct
     public void validate()
     {

diff --git a/.../trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueMetastoreModule.java b/.../trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/v1/GlueMetastoreModule.java
@@ -63,6 +63,9 @@ protected void setup(Binder binder)
         glueConfig.getCatalogId().ifPresent(catalogId -> requestHandlers.addBinding().toInstance(new GlueCatalogIdRequestHandler(catalogId)));
         glueConfig.getGlueProxyApiId().ifPresent(glueProxyApiId -> requestHandlers.addBinding()
                 .toInstance(new ProxyApiRequestHandler(glueProxyApiId)));
+        if (glueConfig.isSkipArchive()) {
+            requestHandlers.addBinding().toInstance(new SkipArchiveRequestHandler());
+        }
         binder.bind(AWSCredentialsProvider.class).toProvider(GlueCredentialsProvider.class).in(Scopes.SINGLETON);
 
         newOptionalBinder(binder, Key.get(new TypeLiteral<Predicate<Table>>() {}, ForGlueHiveMetastore.class))

diff --git a/...talog/glue/SkipArchiveRequestHandler.java → ...re/glue/v1/SkipArchiveRequestHandler.java b/...talog/glue/SkipArchiveRequestHandler.java → ...re/glue/v1/SkipArchiveRequestHandler.java
@@ -11,7 +11,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.trino.plugin.iceberg.catalog.glue;
+package io.trino.plugin.hive.metastore.glue.v1;
 
 import com.amazonaws.AmazonWebServiceRequest;
 import com.amazonaws.handlers.RequestHandler2;

diff --git a/...ive/src/test/java/io/trino/plugin/hive/metastore/glue/v1/TestGlueHiveMetastoreConfig.java b/...ive/src/test/java/io/trino/plugin/hive/metastore/glue/v1/TestGlueHiveMetastoreConfig.java
@@ -46,6 +46,7 @@ public void testDefaults()
                 .setPartitionSegments(5)
                 .setGetPartitionThreads(20)
                 .setAssumeCanonicalPartitionKeys(false)
+                .setSkipArchive(false)
                 .setReadStatisticsThreads(5)
                 .setWriteStatisticsThreads(20));
     }
@@ -72,6 +73,7 @@ public void testExplicitPropertyMapping()
                 .put("hive.metastore.glue.partitions-segments", "10")
                 .put("hive.metastore.glue.get-partition-threads", "42")
                 .put("hive.metastore.glue.assume-canonical-partition-keys", "true")
+                .put("hive.metastore.glue.skip-archive", "true")
                 .put("hive.metastore.glue.read-statistics-threads", "42")
                 .put("hive.metastore.glue.write-statistics-threads", "43")
                 .buildOrThrow();
@@ -95,6 +97,7 @@ public void testExplicitPropertyMapping()
                 .setPartitionSegments(10)
                 .setGetPartitionThreads(42)
                 .setAssumeCanonicalPartitionKeys(true)
+                .setSkipArchive(true)
                 .setReadStatisticsThreads(42)
                 .setWriteStatisticsThreads(43);
 

diff --git a/...-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/IcebergGlueCatalogModule.java b/...-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/IcebergGlueCatalogModule.java
@@ -28,6 +28,7 @@
 import io.trino.plugin.hive.metastore.glue.v1.GlueCredentialsProvider;
 import io.trino.plugin.hive.metastore.glue.v1.GlueHiveMetastoreConfig;
 import io.trino.plugin.hive.metastore.glue.v1.GlueMetastoreModule;
+import io.trino.plugin.hive.metastore.glue.v1.SkipArchiveRequestHandler;
 import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider;
 import io.trino.plugin.iceberg.catalog.TrinoCatalogFactory;
 import io.trino.plugin.iceberg.procedure.MigrateProcedure;
@@ -37,7 +38,6 @@
 
 import static com.google.inject.multibindings.Multibinder.newSetBinder;
 import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
-import static io.airlift.configuration.ConditionalModule.conditionalModule;
 import static io.airlift.configuration.ConfigBinder.configBinder;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
@@ -56,10 +56,11 @@ protected void setup(Binder binder)
         binder.bind(TrinoCatalogFactory.class).to(TrinoGlueCatalogFactory.class).in(Scopes.SINGLETON);
         newExporter(binder).export(TrinoCatalogFactory.class).withGeneratedName();
 
-        install(conditionalModule(
-                IcebergGlueCatalogConfig.class,
-                IcebergGlueCatalogConfig::isSkipArchive,
-                internalBinder -> newSetBinder(internalBinder, RequestHandler2.class, ForGlueHiveMetastore.class).addBinding().toInstance(new SkipArchiveRequestHandler())));
+        GlueHiveMetastoreConfig glueHiveConfig = buildConfigObject(GlueHiveMetastoreConfig.class);
+        IcebergGlueCatalogConfig glueIcebergConfig = buildConfigObject(IcebergGlueCatalogConfig.class);
+        if (glueIcebergConfig.isSkipArchive() || !glueHiveConfig.isSkipArchive()) {
+            newSetBinder(binder, RequestHandler2.class, ForGlueHiveMetastore.class).addBinding().toInstance(new SkipArchiveRequestHandler());
+        }
 
         // Required to inject HiveMetastoreFactory for migrate procedure
         binder.bind(Key.get(boolean.class, HideDeltaLakeTables.class)).toInstance(false);

diff --git a/...src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java b/...src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogSkipArchive.java
@@ -82,6 +82,8 @@ protected QueryRunner createQueryRunner()
                 .setIcebergProperties(
                         ImmutableMap.<String, String>builder()
                                 .put("iceberg.catalog.type", "glue")
+                                // this is not required, since iceberg.glue.skip-archive is true by default, but verify that Guice modules load correctly
+                                .put("hive.metastore.glue.skip-archive", "true")
                                 .put("hive.metastore.glue.default-warehouse-dir", schemaDirectory.getAbsolutePath())
                                 .buildOrThrow())
                 .setSchemaInitializer(

diff --git a/...g/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestingIcebergGlueCatalogModule.java b/...g/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestingIcebergGlueCatalogModule.java
@@ -27,6 +27,7 @@
 import io.trino.plugin.hive.metastore.glue.v1.GlueCredentialsProvider;
 import io.trino.plugin.hive.metastore.glue.v1.GlueHiveMetastoreConfig;
 import io.trino.plugin.hive.metastore.glue.v1.GlueMetastoreModule;
+import io.trino.plugin.hive.metastore.glue.v1.SkipArchiveRequestHandler;
 import io.trino.plugin.iceberg.catalog.IcebergTableOperationsProvider;
 import io.trino.plugin.iceberg.catalog.TrinoCatalogFactory;