From 8f02178e2b80bc3a9330c2f036133416b8185c39 Mon Sep 17 00:00:00 2001 From: Shivam Malhotra Date: Fri, 18 Oct 2024 18:03:59 -0500 Subject: [PATCH] fix: For iceberg test failures with unclosed ResolvingFileIO instance (#6217) --- .../iceberg/util/IcebergToolsTest.java | 4 ++-- .../TestCatalog/IcebergTestCatalog.java | 17 +++++++++++++-- .../iceberg/TestCatalog/IcebergTestTable.java | 21 +++++++------------ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java index eb1640f07c2..81e3b729b36 100644 --- a/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java +++ b/extensions/iceberg/s3/src/test/java/io/deephaven/iceberg/util/IcebergToolsTest.java @@ -14,7 +14,6 @@ import io.deephaven.iceberg.TestCatalog.IcebergTestCatalog; import io.deephaven.test.types.OutOfBandTest; import org.apache.iceberg.Snapshot; -import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.junit.After; @@ -109,7 +108,7 @@ public abstract class IcebergToolsTest { private final List keys = new ArrayList<>(); private String warehousePath; - private Catalog resourceCatalog; + private IcebergTestCatalog resourceCatalog; @Rule public final EngineCleanup framework = new EngineCleanup(); @@ -134,6 +133,7 @@ public void setUp() throws ExecutionException, InterruptedException { @After public void tearDown() throws ExecutionException, InterruptedException { + resourceCatalog.close(); for (String key : keys) { asyncClient.deleteObject(DeleteObjectRequest.builder().bucket(bucket).key(key).build()).get(); } diff --git a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java index 3d95032e1f8..75fffc8cf9d 100644 --- a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java +++ b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestCatalog.java @@ -3,6 +3,7 @@ // package io.deephaven.iceberg.TestCatalog; +import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.*; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; @@ -10,18 +11,25 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.io.ResolvingFileIO; import org.jetbrains.annotations.NotNull; import java.io.File; import java.util.*; -public class IcebergTestCatalog implements Catalog, SupportsNamespaces { +public class IcebergTestCatalog implements Catalog, SupportsNamespaces, AutoCloseable { private final Map> namespaceTableMap; private final Map tableMap; + private final ResolvingFileIO fileIO; + private IcebergTestCatalog(final String path, @NotNull final Map properties) { namespaceTableMap = new HashMap<>(); tableMap = new HashMap<>(); + final Configuration hadoopConf = new Configuration(); + fileIO = new ResolvingFileIO(); + fileIO.setConf(hadoopConf); + fileIO.initialize(properties); // Assume first level is namespace. final File root = new File(path); @@ -33,7 +41,7 @@ private IcebergTestCatalog(final String path, @NotNull final Map if (tableFile.isDirectory()) { // Second level is table name. final TableIdentifier tableId = TableIdentifier.of(namespace, tableFile.getName()); - final Table table = IcebergTestTable.loadFromMetadata(tableFile.getAbsolutePath(), properties); + final Table table = IcebergTestTable.loadFromMetadata(tableFile.getAbsolutePath(), fileIO); // Add it to the maps. namespaceTableMap.get(namespace).put(tableId, table); @@ -103,4 +111,9 @@ public boolean setProperties(Namespace namespace, Map map) throw public boolean removeProperties(Namespace namespace, Set set) throws NoSuchNamespaceException { return false; } + + @Override + public void close() { + fileIO.close(); + } } diff --git a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java index 1ae6894bd13..f64e1af71ad 100644 --- a/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java +++ b/extensions/iceberg/src/test/java/io/deephaven/iceberg/TestCatalog/IcebergTestTable.java @@ -3,12 +3,10 @@ // package io.deephaven.iceberg.TestCatalog; -import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.*; import org.apache.iceberg.encryption.EncryptionManager; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.LocationProvider; -import org.apache.iceberg.io.ResolvingFileIO; import org.jetbrains.annotations.NotNull; import java.io.File; @@ -19,12 +17,12 @@ public class IcebergTestTable implements Table { private final TableMetadata metadata; - private final Map properties; - private final Configuration hadoopConf; + private final FileIO fileIO; - private IcebergTestTable(@NotNull final String path, @NotNull final Map properties) { - this.properties = properties; - hadoopConf = new Configuration(); + private IcebergTestTable( + @NotNull final String path, + @NotNull final FileIO fileIO) { + this.fileIO = fileIO; final File metadataRoot = new File(path, "metadata"); @@ -50,8 +48,8 @@ private IcebergTestTable(@NotNull final String path, @NotNull final Map properties) { - return new IcebergTestTable(path, properties); + @NotNull final FileIO fileIO) { + return new IcebergTestTable(path, fileIO); } @Override @@ -220,10 +218,7 @@ public Transaction newTransaction() { @Override public FileIO io() { - final ResolvingFileIO io = new ResolvingFileIO(); - io.setConf(hadoopConf); - io.initialize(properties); - return io; + return fileIO; } @Override