From d0a716f7e964a785ad70cbc445acf86654a04392 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Sat, 11 May 2024 14:38:56 +0200 Subject: [PATCH] Properly close Glue client in tests Fix potential resource leak during tests' execution. --- .../TestDeltaLakeSharedGlueMetastoreViews.java | 2 +- ...LakeSharedGlueMetastoreWithTableRedirections.java | 2 +- ...akeTableWithCustomLocationUsingGlueMetastore.java | 2 +- ...DeltaLakeConcurrentModificationGlueMetastore.java | 2 +- .../TestDeltaLakeRegisterTableProcedureWithGlue.java | 2 +- .../glue/TestDeltaLakeViewsGlueMetastore.java | 2 +- .../glue/TestDeltaS3AndGlueMetastoreTest.java | 2 +- .../TestHiveConcurrentModificationGlueMetastore.java | 2 +- .../plugin/hive/metastore/TestGlueHiveMetastore.java | 7 +++++-- .../metastore/glue/TestingGlueHiveMetastore.java | 12 ++++++++---- .../TestIcebergGlueTableOperationsInsertFailure.java | 2 +- .../glue/TestIcebergS3AndGlueMetastoreTest.java | 2 +- .../catalog/glue/TestSharedGlueMetastore.java | 2 +- 13 files changed, 24 insertions(+), 17 deletions(-) diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreViews.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreViews.java index b12295bdd63b6..1dca66a12ed6a 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreViews.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreViews.java @@ -29,6 +29,6 @@ public class TestDeltaLakeSharedGlueMetastoreViews @Override protected HiveMetastore createTestMetastore(Path dataDirectory) { - return createTestingGlueHiveMetastore(dataDirectory); + return createTestingGlueHiveMetastore(dataDirectory, this::closeAfterClass); } } diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreWithTableRedirections.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreWithTableRedirections.java index 59f8800d3ac60..4bf2c695bed27 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreWithTableRedirections.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSharedGlueMetastoreWithTableRedirections.java @@ -65,7 +65,7 @@ protected QueryRunner createQueryRunner() .put("delta.hive-catalog-name", "hive_with_redirections") .buildOrThrow()); - this.glueMetastore = createTestingGlueHiveMetastore(dataDirectory); + this.glueMetastore = createTestingGlueHiveMetastore(dataDirectory, this::closeAfterClass); queryRunner.installPlugin(new TestingHivePlugin(queryRunner.getCoordinator().getBaseDataDir().resolve("hive_data"), glueMetastore)); queryRunner.createCatalog( "hive_with_redirections", diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeTableWithCustomLocationUsingGlueMetastore.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeTableWithCustomLocationUsingGlueMetastore.java index 06b700c6de0fe..8deee1b2a95e0 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeTableWithCustomLocationUsingGlueMetastore.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeTableWithCustomLocationUsingGlueMetastore.java @@ -41,7 +41,7 @@ protected QueryRunner createQueryRunner() { Path warehouseDir = Files.createTempDirectory("warehouse-dir"); closeAfterClass(() -> deleteRecursively(warehouseDir, ALLOW_INSECURE)); - metastore = createTestingGlueHiveMetastore(warehouseDir); + metastore = createTestingGlueHiveMetastore(warehouseDir, this::closeAfterClass); schema = "test_tables_with_custom_location" + randomNameSuffix(); return DeltaLakeQueryRunner.builder(schema) .addDeltaProperty("hive.metastore", "glue") diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeConcurrentModificationGlueMetastore.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeConcurrentModificationGlueMetastore.java index 1eef47e416cd3..c44b3865ff07b 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeConcurrentModificationGlueMetastore.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeConcurrentModificationGlueMetastore.java @@ -75,7 +75,7 @@ protected QueryRunner createQueryRunner() GlueHiveMetastoreConfig glueConfig = new GlueHiveMetastoreConfig() .setDefaultWarehouseDir(dataDirectory.toUri().toString()); - GlueClient glueClient = createGlueClient(new GlueHiveMetastoreConfig(), OpenTelemetry.noop()); + GlueClient glueClient = closeAfterClass(createGlueClient(new GlueHiveMetastoreConfig(), OpenTelemetry.noop())); GlueClient proxiedGlueClient = newProxy(GlueClient.class, (proxy, method, args) -> { Object result; try { diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeRegisterTableProcedureWithGlue.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeRegisterTableProcedureWithGlue.java index 4b10f7c3ba0fd..d190234723a63 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeRegisterTableProcedureWithGlue.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeRegisterTableProcedureWithGlue.java @@ -40,7 +40,7 @@ protected QueryRunner createQueryRunner() { Path warehouseDir = Files.createTempDirectory("warehouse-dir"); closeAfterClass(() -> deleteRecursively(warehouseDir, ALLOW_INSECURE)); - metastore = createTestingGlueHiveMetastore(warehouseDir); + metastore = createTestingGlueHiveMetastore(warehouseDir, this::closeAfterClass); schema = "test_delta_lake_register_table" + randomNameSuffix(); return DeltaLakeQueryRunner.builder(schema) .addDeltaProperty("hive.metastore", "glue") diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeViewsGlueMetastore.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeViewsGlueMetastore.java index 360cdfad38526..f5b694bbf34c2 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeViewsGlueMetastore.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaLakeViewsGlueMetastore.java @@ -48,7 +48,7 @@ protected QueryRunner createQueryRunner() { Path warehouseDir = Files.createTempDirectory("warehouse-dir"); closeAfterClass(() -> deleteRecursively(warehouseDir, ALLOW_INSECURE)); - metastore = createTestingGlueHiveMetastore(warehouseDir); + metastore = createTestingGlueHiveMetastore(warehouseDir, this::closeAfterClass); schema = "test_delta_lake_glue_views_" + randomNameSuffix(); return DeltaLakeQueryRunner.builder(schema) .addDeltaProperty("hive.metastore", "glue") diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java index e64131772e52c..865857a328514 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/metastore/glue/TestDeltaS3AndGlueMetastoreTest.java @@ -40,7 +40,7 @@ public TestDeltaS3AndGlueMetastoreTest() protected QueryRunner createQueryRunner() throws Exception { - metastore = createTestingGlueHiveMetastore(URI.create(schemaPath())); + metastore = createTestingGlueHiveMetastore(URI.create(schemaPath()), this::closeAfterClass); return DeltaLakeQueryRunner.builder(schemaName) .setDeltaProperties(ImmutableMap.builder() .put("hive.metastore", "glue") diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConcurrentModificationGlueMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConcurrentModificationGlueMetastore.java index 7f5c3b296fda1..29f2ff6430a65 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConcurrentModificationGlueMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConcurrentModificationGlueMetastore.java @@ -73,7 +73,7 @@ protected QueryRunner createQueryRunner() GlueHiveMetastoreConfig glueConfig = new GlueHiveMetastoreConfig() .setDefaultWarehouseDir(dataDirectory.toUri().toString()); - GlueClient glueClient = createGlueClient(new GlueHiveMetastoreConfig(), OpenTelemetry.noop()); + GlueClient glueClient = closeAfterClass(createGlueClient(new GlueHiveMetastoreConfig(), OpenTelemetry.noop())); GlueClient proxiedGlueClient = newProxy(GlueClient.class, (proxy, method, args) -> { try { if (method.getName().equals("updateTable")) { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/TestGlueHiveMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/TestGlueHiveMetastore.java index f7da170b8ddf9..05f210254d18b 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/TestGlueHiveMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/TestGlueHiveMetastore.java @@ -13,6 +13,7 @@ */ package io.trino.plugin.hive.metastore; +import io.trino.plugin.base.util.AutoCloseableCloser; import io.trino.plugin.hive.metastore.glue.GlueHiveMetastore; import org.junit.jupiter.api.AfterAll; @@ -27,6 +28,7 @@ final class TestGlueHiveMetastore extends AbstractTestHiveMetastore { + private final AutoCloseableCloser closer = AutoCloseableCloser.create(); private final Path tempDir; private final GlueHiveMetastore metastore; @@ -34,15 +36,16 @@ final class TestGlueHiveMetastore throws IOException { tempDir = createTempDirectory("test"); - metastore = createTestingGlueHiveMetastore(tempDir); + metastore = createTestingGlueHiveMetastore(tempDir, closer::register); } @AfterAll void tearDown() - throws IOException + throws Exception { metastore.shutdown(); deleteRecursively(tempDir, ALLOW_INSECURE); + closer.close(); } @Override diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingGlueHiveMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingGlueHiveMetastore.java index a499cfad7c222..40eeb96b08947 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingGlueHiveMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestingGlueHiveMetastore.java @@ -15,11 +15,13 @@ import io.opentelemetry.api.OpenTelemetry; import io.trino.plugin.hive.metastore.glue.GlueHiveMetastore.TableKind; +import software.amazon.awssdk.services.glue.GlueClient; import java.io.IOException; import java.net.URI; import java.nio.file.Path; import java.util.EnumSet; +import java.util.function.Consumer; import static com.google.common.base.Verify.verify; import static io.trino.plugin.hive.HiveTestUtils.HDFS_FILE_SYSTEM_FACTORY; @@ -32,7 +34,7 @@ public final class TestingGlueHiveMetastore { private TestingGlueHiveMetastore() {} - public static GlueHiveMetastore createTestingGlueHiveMetastore(Path defaultWarehouseDir) + public static GlueHiveMetastore createTestingGlueHiveMetastore(Path defaultWarehouseDir, Consumer registerResource) { if (!exists(defaultWarehouseDir)) { try { @@ -43,15 +45,17 @@ public static GlueHiveMetastore createTestingGlueHiveMetastore(Path defaultWareh } } verify(isDirectory(defaultWarehouseDir), "%s is not a directory", defaultWarehouseDir); - return createTestingGlueHiveMetastore(defaultWarehouseDir.toUri()); + return createTestingGlueHiveMetastore(defaultWarehouseDir.toUri(), registerResource); } - public static GlueHiveMetastore createTestingGlueHiveMetastore(URI warehouseUri) + public static GlueHiveMetastore createTestingGlueHiveMetastore(URI warehouseUri, Consumer registerResource) { GlueHiveMetastoreConfig glueConfig = new GlueHiveMetastoreConfig() .setDefaultWarehouseDir(warehouseUri.toString()); + GlueClient glueClient = createGlueClient(glueConfig, OpenTelemetry.noop()); + registerResource.accept(glueClient); return new GlueHiveMetastore( - createGlueClient(glueConfig, OpenTelemetry.noop()), + glueClient, new GlueContext(glueConfig), GlueCache.NOOP, HDFS_FILE_SYSTEM_FACTORY, diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueTableOperationsInsertFailure.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueTableOperationsInsertFailure.java index 2e73e4d084b02..5b2ed8f8ec600 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueTableOperationsInsertFailure.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueTableOperationsInsertFailure.java @@ -91,7 +91,7 @@ protected QueryRunner createQueryRunner() queryRunner.installPlugin(new TestingIcebergPlugin(dataDirectory, Optional.of(new TestingIcebergGlueCatalogModule(awsGlueAsyncAdapterProvider)))); queryRunner.createCatalog(ICEBERG_CATALOG, "iceberg", ImmutableMap.of()); - glueHiveMetastore = createTestingGlueHiveMetastore(dataDirectory); + glueHiveMetastore = createTestingGlueHiveMetastore(dataDirectory, this::closeAfterClass); Database database = Database.builder() .setDatabaseName(schemaName) diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java index c0d80e7c7111a..6211bf406326c 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergS3AndGlueMetastoreTest.java @@ -44,7 +44,7 @@ public TestIcebergS3AndGlueMetastoreTest() protected QueryRunner createQueryRunner() throws Exception { - metastore = createTestingGlueHiveMetastore(URI.create(schemaPath())); + metastore = createTestingGlueHiveMetastore(URI.create(schemaPath()), this::closeAfterClass); return IcebergQueryRunner.builder() .setIcebergProperties(ImmutableMap.builder() .put("iceberg.catalog.type", "glue") diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestSharedGlueMetastore.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestSharedGlueMetastore.java index 0cfbfb9e57885..1635bac676339 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestSharedGlueMetastore.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestSharedGlueMetastore.java @@ -93,7 +93,7 @@ protected QueryRunner createQueryRunner() "hive.metastore.glue.default-warehouse-dir", dataDirectory.toString(), "iceberg.hive-catalog-name", "hive")); - this.glueMetastore = createTestingGlueHiveMetastore(dataDirectory); + this.glueMetastore = createTestingGlueHiveMetastore(dataDirectory, this::closeAfterClass); queryRunner.installPlugin(new TestingHivePlugin(queryRunner.getCoordinator().getBaseDataDir().resolve("hive_data"), glueMetastore)); queryRunner.createCatalog(HIVE_CATALOG, "hive"); queryRunner.createCatalog(