diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java index e46380191781..af5f7ce7335a 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java @@ -23,9 +23,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import java.io.IOException; import java.nio.file.Path; +import java.util.Collections; +import java.util.concurrent.TimeUnit; import org.apache.iceberg.AppendFiles; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.PartitionSpec; @@ -41,14 +44,17 @@ import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.types.Types; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -public class HiveCreateReplaceTableTest extends HiveMetastoreTest { +public class HiveCreateReplaceTableTest { + private static final String DB_NAME = "hivedb"; private static final String TABLE_NAME = "tbl"; private static final TableIdentifier TABLE_IDENTIFIER = TableIdentifier.of(DB_NAME, TABLE_NAME); private static final Schema SCHEMA = @@ -60,8 +66,27 @@ public class HiveCreateReplaceTableTest extends HiveMetastoreTest { private String tableLocation; + @RegisterExtension + private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = + new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + + private static HiveCatalog catalog; + + @BeforeAll + public static void initCatalog() { + catalog = + (HiveCatalog) + CatalogUtil.loadCatalog( + HiveCatalog.class.getName(), + CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE, + ImmutableMap.of( + CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, + String.valueOf(TimeUnit.SECONDS.toMillis(10))), + HIVE_METASTORE_EXTENSION.hiveConf()); + } + @BeforeEach - public void createTableLocation() throws IOException { + public void createTableLocation() { tableLocation = temp.resolve("hive-").toString(); } diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java new file mode 100644 index 000000000000..552307395c59 --- /dev/null +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.iceberg.hive; + +import java.util.Map; +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +public class HiveMetastoreExtension implements BeforeAllCallback, AfterAllCallback { + private HiveMetaStoreClient metastoreClient; + private TestHiveMetastore metastore; + private final Map hiveConfOverride; + private final String databaseName; + + public HiveMetastoreExtension(String databaseName, Map hiveConfOverride) { + this.databaseName = databaseName; + this.hiveConfOverride = hiveConfOverride; + } + + @Override + public void beforeAll(ExtensionContext extensionContext) throws Exception { + metastore = new TestHiveMetastore(); + HiveConf hiveConfWithOverrides = new HiveConf(TestHiveMetastore.class); + if (hiveConfOverride != null) { + for (Map.Entry kv : hiveConfOverride.entrySet()) { + hiveConfWithOverrides.set(kv.getKey(), kv.getValue()); + } + } + + metastore.start(hiveConfWithOverrides); + metastoreClient = new HiveMetaStoreClient(hiveConfWithOverrides); + String dbPath = metastore.getDatabasePath(databaseName); + Database db = new Database(databaseName, "description", dbPath, Maps.newHashMap()); + metastoreClient.createDatabase(db); + } + + @Override + public void afterAll(ExtensionContext extensionContext) throws Exception { + if (null != metastoreClient) { + metastoreClient.close(); + } + + if (null != metastore) { + metastore.stop(); + } + + metastoreClient = null; + metastore = null; + } + + public HiveMetaStoreClient metastoreClient() { + return metastoreClient; + } + + public HiveConf hiveConf() { + return metastore.hiveConf(); + } + + public TestHiveMetastore metastore() { + return metastore; + } +} diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreTest.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreTest.java deleted file mode 100644 index e48df0ce9378..000000000000 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreTest.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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 org.apache.iceberg.hive; - -import java.util.Collections; -import java.util.Map; -import java.util.concurrent.TimeUnit; -import org.apache.hadoop.hive.conf.HiveConf; -import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; -import org.apache.hadoop.hive.metastore.api.Database; -import org.apache.iceberg.CatalogProperties; -import org.apache.iceberg.CatalogUtil; -import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; -import org.apache.iceberg.relocated.com.google.common.collect.Maps; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; - -public abstract class HiveMetastoreTest { - - protected static final String DB_NAME = "hivedb"; - protected static final long EVICTION_INTERVAL = TimeUnit.SECONDS.toMillis(10); - - protected static HiveMetaStoreClient metastoreClient; - protected static HiveCatalog catalog; - protected static HiveConf hiveConf; - protected static TestHiveMetastore metastore; - - @BeforeAll - public static void startMetastore() throws Exception { - startMetastore(Collections.emptyMap()); - } - - public static void startMetastore(Map hiveConfOverride) throws Exception { - HiveMetastoreTest.metastore = new TestHiveMetastore(); - HiveConf hiveConfWithOverrides = new HiveConf(TestHiveMetastore.class); - if (hiveConfOverride != null) { - for (Map.Entry kv : hiveConfOverride.entrySet()) { - hiveConfWithOverrides.set(kv.getKey(), kv.getValue()); - } - } - - metastore.start(hiveConfWithOverrides); - HiveMetastoreTest.hiveConf = metastore.hiveConf(); - HiveMetastoreTest.metastoreClient = new HiveMetaStoreClient(hiveConfWithOverrides); - String dbPath = metastore.getDatabasePath(DB_NAME); - Database db = new Database(DB_NAME, "description", dbPath, Maps.newHashMap()); - metastoreClient.createDatabase(db); - HiveMetastoreTest.catalog = - (HiveCatalog) - CatalogUtil.loadCatalog( - HiveCatalog.class.getName(), - CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE, - ImmutableMap.of( - CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, - String.valueOf(EVICTION_INTERVAL)), - hiveConfWithOverrides); - } - - @AfterAll - public static void stopMetastore() throws Exception { - HiveMetastoreTest.catalog = null; - - metastoreClient.close(); - HiveMetastoreTest.metastoreClient = null; - - metastore.stop(); - HiveMetastoreTest.metastore = null; - } -} diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java index 51f4b5953276..9dd193271ad6 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java @@ -26,22 +26,36 @@ import java.io.File; import java.nio.file.Paths; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.hadoop.fs.Path; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.types.Types; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.RegisterExtension; -public class HiveTableBaseTest extends HiveMetastoreTest { +public class HiveTableBaseTest { static final String TABLE_NAME = "tbl"; + static final String DB_NAME = "hivedb"; static final TableIdentifier TABLE_IDENTIFIER = TableIdentifier.of(DB_NAME, TABLE_NAME); + @RegisterExtension + protected static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = + new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + + protected static HiveCatalog catalog; + static final Schema schema = new Schema(Types.StructType.of(required(1, "id", Types.LongType.get())).fields()); @@ -56,6 +70,19 @@ public class HiveTableBaseTest extends HiveMetastoreTest { private Path tableLocation; + @BeforeAll + public static void initCatalog() { + catalog = + (HiveCatalog) + CatalogUtil.loadCatalog( + HiveCatalog.class.getName(), + CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE, + ImmutableMap.of( + CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, + String.valueOf(TimeUnit.SECONDS.toMillis(10))), + HIVE_METASTORE_EXTENSION.hiveConf()); + } + @BeforeEach public void createTestTable() { this.tableLocation = @@ -65,12 +92,12 @@ public void createTestTable() { @AfterEach public void dropTestTable() throws Exception { // drop the table data - tableLocation.getFileSystem(hiveConf).delete(tableLocation, true); + tableLocation.getFileSystem(HIVE_METASTORE_EXTENSION.hiveConf()).delete(tableLocation, true); catalog.dropTable(TABLE_IDENTIFIER, false /* metadata only, location was already deleted */); } private static String getTableBasePath(String tableName) { - String databasePath = metastore.getDatabasePath(DB_NAME); + String databasePath = HIVE_METASTORE_EXTENSION.metastore().getDatabasePath(DB_NAME); return Paths.get(databasePath, tableName).toAbsolutePath().toString(); } diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableTest.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableTest.java index 0b5edf21aec7..0fa6c94bf154 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableTest.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableTest.java @@ -91,7 +91,9 @@ public void testCreate() throws TException { // Table should be renamed in hive metastore String tableName = TABLE_IDENTIFIER.name(); org.apache.hadoop.hive.metastore.api.Table table = - metastoreClient.getTable(TABLE_IDENTIFIER.namespace().level(0), tableName); + HIVE_METASTORE_EXTENSION + .metastoreClient() + .getTable(TABLE_IDENTIFIER.namespace().level(0), tableName); // check parameters are in expected state Map parameters = table.getParameters(); @@ -255,7 +257,7 @@ public void testExistingTableUpdate() throws TException { assertThat(icebergTable.schema().asStruct()).isEqualTo(altered.asStruct()); final org.apache.hadoop.hive.metastore.api.Table table = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); final List hiveColumns = table.getSd().getCols().stream().map(FieldSchema::getName).collect(Collectors.toList()); final List icebergColumns = @@ -309,10 +311,10 @@ public void testColumnTypeChangeInMetastore() throws TException { public void testFailure() throws TException { Table icebergTable = catalog.loadTable(TABLE_IDENTIFIER); org.apache.hadoop.hive.metastore.api.Table table = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); String dummyLocation = "dummylocation"; table.getParameters().put(METADATA_LOCATION_PROP, dummyLocation); - metastoreClient.alter_table(DB_NAME, TABLE_NAME, table); + HIVE_METASTORE_EXTENSION.metastoreClient().alter_table(DB_NAME, TABLE_NAME, table); assertThatThrownBy( () -> icebergTable.updateSchema().addColumn("data", Types.LongType.get()).commit()) .isInstanceOf(CommitFailedException.class) @@ -333,7 +335,7 @@ public void testListTables() throws TException, IOException { // create a hive table String hiveTableName = "test_hive_table"; org.apache.hadoop.hive.metastore.api.Table hiveTable = createHiveTable(hiveTableName); - metastoreClient.createTable(hiveTable); + HIVE_METASTORE_EXTENSION.metastoreClient().createTable(hiveTable); catalog.setListAllTables(false); List tableIdents1 = catalog.listTables(TABLE_IDENTIFIER.namespace()); @@ -344,7 +346,7 @@ public void testListTables() throws TException, IOException { assertThat(tableIdents2).as("should be 2 tables in namespace .").hasSize(2); assertThat(catalog.tableExists(TABLE_IDENTIFIER)).isTrue(); - metastoreClient.dropTable(DB_NAME, hiveTableName); + HIVE_METASTORE_EXTENSION.metastoreClient().dropTable(DB_NAME, hiveTableName); } private org.apache.hadoop.hive.metastore.api.Table createHiveTable(String hiveTableName) @@ -410,13 +412,13 @@ public void testNonDefaultDatabaseLocation() throws IOException, TException { assertThat(table.location()).isEqualTo(namespaceMeta.get("location") + "/" + TABLE_NAME); // Drop the database and purge the files - metastoreClient.dropDatabase(NON_DEFAULT_DATABASE, true, true, true); + HIVE_METASTORE_EXTENSION.metastoreClient().dropDatabase(NON_DEFAULT_DATABASE, true, true, true); } @Test public void testRegisterTable() throws TException { org.apache.hadoop.hive.metastore.api.Table originalTable = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); Map originalParams = originalTable.getParameters(); assertThat(originalParams).isNotNull(); @@ -432,7 +434,7 @@ public void testRegisterTable() throws TException { catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0)); org.apache.hadoop.hive.metastore.api.Table newTable = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); Map newTableParameters = newTable.getParameters(); assertThat(newTableParameters) @@ -466,7 +468,7 @@ public void testRegisterHadoopTableToHiveCatalog() throws IOException, TExceptio .collect(Collectors.toList()); assertThat(metadataFiles).hasSize(2); - assertThatThrownBy(() -> metastoreClient.getTable(DB_NAME, "table1")) + assertThatThrownBy(() -> HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, "table1")) .isInstanceOf(NoSuchObjectException.class) .hasMessage("hivedb.table1 table not found"); assertThatThrownBy(() -> catalog.loadTable(identifier)) @@ -476,7 +478,7 @@ public void testRegisterHadoopTableToHiveCatalog() throws IOException, TExceptio // register the table to hive catalog using the latest metadata file String latestMetadataFile = ((BaseTable) table).operations().current().metadataFileLocation(); catalog.registerTable(identifier, "file:" + latestMetadataFile); - assertThat(metastoreClient.getTable(DB_NAME, "table1")).isNotNull(); + assertThat(HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, "table1")).isNotNull(); // load the table in hive catalog table = catalog.loadTable(identifier); @@ -523,7 +525,7 @@ private String appendData(Table table, String fileName) throws IOException { @Test public void testRegisterExistingTable() throws TException { org.apache.hadoop.hive.metastore.api.Table originalTable = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); Map originalParams = originalTable.getParameters(); assertThat(originalParams).isNotNull(); @@ -550,7 +552,7 @@ public void testEngineHiveEnabledDefault() throws TException { catalog.createTable(TABLE_IDENTIFIER, schema, PartitionSpec.unpartitioned()); org.apache.hadoop.hive.metastore.api.Table hmsTable = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); assertHiveEnabled(hmsTable, false); } @@ -565,7 +567,7 @@ public void testEngineHiveEnabledConfig() throws TException { catalog.createTable(TABLE_IDENTIFIER, schema, PartitionSpec.unpartitioned()); org.apache.hadoop.hive.metastore.api.Table hmsTable = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); assertHiveEnabled(hmsTable, true); @@ -575,7 +577,7 @@ public void testEngineHiveEnabledConfig() throws TException { catalog.getConf().set(ConfigProperties.ENGINE_HIVE_ENABLED, "false"); catalog.createTable(TABLE_IDENTIFIER, schema, PartitionSpec.unpartitioned()); - hmsTable = metastoreClient.getTable(DB_NAME, TABLE_NAME); + hmsTable = HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); assertHiveEnabled(hmsTable, false); } @@ -592,7 +594,7 @@ public void testEngineHiveEnabledTableProperty() throws TException { catalog.createTable(TABLE_IDENTIFIER, schema, PartitionSpec.unpartitioned(), tableProperties); org.apache.hadoop.hive.metastore.api.Table hmsTable = - metastoreClient.getTable(DB_NAME, TABLE_NAME); + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); assertHiveEnabled(hmsTable, true); @@ -603,7 +605,7 @@ public void testEngineHiveEnabledTableProperty() throws TException { catalog.getConf().set(ConfigProperties.ENGINE_HIVE_ENABLED, "true"); catalog.createTable(TABLE_IDENTIFIER, schema, PartitionSpec.unpartitioned(), tableProperties); - hmsTable = metastoreClient.getTable(DB_NAME, TABLE_NAME); + hmsTable = HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, TABLE_NAME); assertHiveEnabled(hmsTable, false); } diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java index 19b9b0effbb4..211fcdbd0c71 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java @@ -28,30 +28,46 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.hive.CachedClientPool.Key; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; -public class TestCachedClientPool extends HiveMetastoreTest { +public class TestCachedClientPool { + private static final long EVICTION_INTERVAL = TimeUnit.SECONDS.toMillis(10); + private static final String DB_NAME = "hivedb"; + + @RegisterExtension + private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = + new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); @Test public void testClientPoolCleaner() throws InterruptedException { - CachedClientPool clientPool = new CachedClientPool(hiveConf, Collections.emptyMap()); + CachedClientPool clientPool = + new CachedClientPool( + HIVE_METASTORE_EXTENSION.hiveConf(), + ImmutableMap.of( + CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, + String.valueOf(EVICTION_INTERVAL))); HiveClientPool clientPool1 = clientPool.clientPool(); assertThat(clientPool1) .isSameAs( CachedClientPool.clientPoolCache() - .getIfPresent(CachedClientPool.extractKey(null, hiveConf))); + .getIfPresent( + CachedClientPool.extractKey(null, HIVE_METASTORE_EXTENSION.hiveConf()))); TimeUnit.MILLISECONDS.sleep(EVICTION_INTERVAL - TimeUnit.SECONDS.toMillis(2)); HiveClientPool clientPool2 = clientPool.clientPool(); assertThat(clientPool2).isSameAs(clientPool1); TimeUnit.MILLISECONDS.sleep(EVICTION_INTERVAL + TimeUnit.SECONDS.toMillis(5)); assertThat( CachedClientPool.clientPoolCache() - .getIfPresent(CachedClientPool.extractKey(null, hiveConf))) + .getIfPresent( + CachedClientPool.extractKey(null, HIVE_METASTORE_EXTENSION.hiveConf()))) .isNull(); // The client has been really closed. @@ -66,6 +82,8 @@ public void testCacheKey() throws Exception { UserGroupInformation foo2 = UserGroupInformation.createProxyUser("foo", current); UserGroupInformation bar = UserGroupInformation.createProxyUser("bar", current); + HiveConf hiveConf = HIVE_METASTORE_EXTENSION.hiveConf(); + Key key1 = foo1.doAs( (PrivilegedAction) diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java index 7ff2bd78a665..49885cc3af2c 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java @@ -37,10 +37,12 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.api.Database; @@ -82,12 +84,14 @@ import org.apache.iceberg.types.Types; import org.apache.iceberg.util.JsonUtil; import org.apache.thrift.TException; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -public class TestHiveCatalog extends HiveMetastoreTest { +public class TestHiveCatalog { private static ImmutableMap meta = ImmutableMap.of( "owner", "apache", @@ -96,6 +100,26 @@ public class TestHiveCatalog extends HiveMetastoreTest { @TempDir private Path temp; + private HiveCatalog catalog; + private static final String DB_NAME = "hivedb"; + + @RegisterExtension + private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = + new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + + @BeforeEach + public void before() { + catalog = + (HiveCatalog) + CatalogUtil.loadCatalog( + HiveCatalog.class.getName(), + CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE, + ImmutableMap.of( + CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, + String.valueOf(TimeUnit.SECONDS.toMillis(10))), + HIVE_METASTORE_EXTENSION.hiveConf()); + } + private Schema getTestSchema() { return new Schema( required(1, "id", Types.IntegerType.get(), "unique ID"), @@ -166,8 +190,8 @@ public void testInitialize() { assertThatNoException() .isThrownBy( () -> { - HiveCatalog catalog = new HiveCatalog(); - catalog.initialize("hive", Maps.newHashMap()); + HiveCatalog hiveCatalog = new HiveCatalog(); + hiveCatalog.initialize("hive", Maps.newHashMap()); }); } @@ -176,8 +200,8 @@ public void testToStringWithoutSetConf() { assertThatNoException() .isThrownBy( () -> { - HiveCatalog catalog = new HiveCatalog(); - catalog.toString(); + HiveCatalog hiveCatalog = new HiveCatalog(); + hiveCatalog.toString(); }); } @@ -186,11 +210,12 @@ public void testInitializeCatalogWithProperties() { Map properties = Maps.newHashMap(); properties.put("uri", "thrift://examplehost:9083"); properties.put("warehouse", "/user/hive/testwarehouse"); - HiveCatalog catalog = new HiveCatalog(); - catalog.initialize("hive", properties); + HiveCatalog hiveCatalog = new HiveCatalog(); + hiveCatalog.initialize("hive", properties); - assertThat(catalog.getConf().get("hive.metastore.uris")).isEqualTo("thrift://examplehost:9083"); - assertThat(catalog.getConf().get("hive.metastore.warehouse.dir")) + assertThat(hiveCatalog.getConf().get("hive.metastore.uris")) + .isEqualTo("thrift://examplehost:9083"); + assertThat(hiveCatalog.getConf().get("hive.metastore.warehouse.dir")) .isEqualTo("/user/hive/testwarehouse"); } @@ -292,7 +317,8 @@ private void createTableAndVerifyOwner( String location = temp.resolve(tbl).toString(); try { Table table = catalog.createTable(tableIdent, schema, spec, location, properties); - org.apache.hadoop.hive.metastore.api.Table hmsTable = metastoreClient.getTable(db, tbl); + org.apache.hadoop.hive.metastore.api.Table hmsTable = + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(db, tbl); assertThat(hmsTable.getOwner()).isEqualTo(owner); Map hmsTableParams = hmsTable.getParameters(); assertThat(hmsTableParams).doesNotContainKey(HiveCatalog.HMS_TABLE_OWNER); @@ -357,7 +383,8 @@ public void testCreateTableCustomSortOrder() throws Exception { public void testCreateNamespace() throws Exception { Namespace namespace1 = Namespace.of("noLocation"); catalog.createNamespace(namespace1, meta); - Database database1 = metastoreClient.getDatabase(namespace1.toString()); + Database database1 = + HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace1.toString()); assertThat(database1.getParameters()).containsEntry("owner", "apache"); assertThat(database1.getParameters()).containsEntry("group", "iceberg"); @@ -380,7 +407,8 @@ public void testCreateNamespace() throws Exception { Namespace namespace2 = Namespace.of("haveLocation"); catalog.createNamespace(namespace2, newMeta); - Database database2 = metastoreClient.getDatabase(namespace2.toString()); + Database database2 = + HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace2.toString()); assertThat(hiveLocalDir) .as("There no same location for db and namespace") .isEqualTo(database2.getLocationUri()); @@ -460,7 +488,7 @@ private void createNamespaceAndVerifyOwnership( Namespace namespace = Namespace.of(name); catalog.createNamespace(namespace, prop); - Database db = metastoreClient.getDatabase(namespace.toString()); + Database db = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace.toString()); assertThat(db.getOwnerName()).isEqualTo(expectedOwner); assertThat(db.getOwnerType()).isEqualTo(expectedOwnerType); @@ -520,7 +548,7 @@ public void testSetNamespaceProperties() throws TException { "location", "file:/data/tmp", "comment", "iceberg test")); - Database database = metastoreClient.getDatabase(namespace.level(0)); + Database database = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace.level(0)); assertThat(database.getParameters()).containsEntry("owner", "alter_apache"); assertThat(database.getParameters()).containsEntry("test", "test"); assertThat(database.getParameters()).containsEntry("group", "iceberg"); @@ -706,7 +734,7 @@ private void setNamespaceOwnershipAndVerify( name, propToCreate, expectedOwnerPostCreate, expectedOwnerTypePostCreate); catalog.setProperties(Namespace.of(name), propToSet); - Database database = metastoreClient.getDatabase(name); + Database database = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(name); assertThat(database.getOwnerName()).isEqualTo(expectedOwnerPostSet); assertThat(database.getOwnerType()).isEqualTo(expectedOwnerTypePostSet); @@ -720,7 +748,7 @@ public void testRemoveNamespaceProperties() throws TException { catalog.removeProperties(namespace, ImmutableSet.of("comment", "owner")); - Database database = metastoreClient.getDatabase(namespace.level(0)); + Database database = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace.level(0)); assertThat(database.getParameters()).doesNotContainKey("owner"); assertThat(database.getParameters()).containsEntry("group", "iceberg"); @@ -852,7 +880,7 @@ private void removeNamespaceOwnershipAndVerify( catalog.removeProperties(Namespace.of(name), propToRemove); - Database database = metastoreClient.getDatabase(name); + Database database = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(name); assertThat(database.getOwnerName()).isEqualTo(expectedOwnerPostRemove); assertThat(database.getOwnerType()).isEqualTo(expectedOwnerTypePostRemove); @@ -922,7 +950,9 @@ public void testTableName() { } private String defaultUri(Namespace namespace) throws TException { - return metastoreClient.getConfigValue("hive.metastore.warehouse.dir", "") + return HIVE_METASTORE_EXTENSION + .metastoreClient() + .getConfigValue("hive.metastore.warehouse.dir", "") + "/" + namespace.level(0) + ".db"; @@ -1098,7 +1128,8 @@ public void testSetCurrentSchema() throws Exception { } private Map hmsTableParameters() throws TException { - org.apache.hadoop.hive.metastore.api.Table hmsTable = metastoreClient.getTable(DB_NAME, "tbl"); + org.apache.hadoop.hive.metastore.api.Table hmsTable = + HIVE_METASTORE_EXTENSION.metastoreClient().getTable(DB_NAME, "tbl"); return hmsTable.getParameters(); } @@ -1131,7 +1162,7 @@ public void testTablePropsDefinedAtCatalogLevel() { HiveCatalog.class.getName(), CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE, catalogProps, - hiveConf); + HIVE_METASTORE_EXTENSION.hiveConf()); try { Table table = @@ -1172,10 +1203,10 @@ public void testDatabaseLocationWithSlashInWarehouseDir() { // With a trailing slash conf.set("hive.metastore.warehouse.dir", "s3://bucket/"); - HiveCatalog catalog = new HiveCatalog(); - catalog.setConf(conf); + HiveCatalog hiveCatalog = new HiveCatalog(); + hiveCatalog.setConf(conf); - Database database = catalog.convertToDatabase(Namespace.of("database"), ImmutableMap.of()); + Database database = hiveCatalog.convertToDatabase(Namespace.of("database"), ImmutableMap.of()); assertThat(database.getLocationUri()).isEqualTo("s3://bucket/database.db"); } diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java index 2d0f3d23ad2a..48946525463c 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java @@ -18,6 +18,8 @@ */ package org.apache.iceberg.hive; +import static org.apache.iceberg.PartitionSpec.builderFor; +import static org.apache.iceberg.types.Types.NestedField.required; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.any; @@ -44,6 +46,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.IntStream; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; import org.apache.hadoop.hive.metastore.IMetaStoreClient; @@ -53,9 +56,14 @@ import org.apache.hadoop.hive.metastore.api.LockState; import org.apache.hadoop.hive.metastore.api.ShowLocksResponse; import org.apache.hadoop.hive.metastore.api.ShowLocksResponseElement; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.HasTableOperations; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.hadoop.ConfigProperties; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -63,15 +71,17 @@ import org.apache.iceberg.types.Types; import org.apache.thrift.TException; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.AdditionalAnswers; import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; import org.mockito.invocation.InvocationOnMock; -public class TestHiveCommitLocks extends HiveTableBaseTest { +public class TestHiveCommitLocks { private static HiveTableOperations spyOps = null; private static HiveClientPool spyClientPool = null; private static CachedClientPool spyCachedClientPool = null; @@ -88,13 +98,35 @@ public class TestHiveCommitLocks extends HiveTableBaseTest { LockResponse notAcquiredLockResponse = new LockResponse(dummyLockId, LockState.NOT_ACQUIRED); ShowLocksResponse emptyLocks = new ShowLocksResponse(Lists.newArrayList()); + private static final String DB_NAME = "hivedb"; + private static final String TABLE_NAME = "tbl"; + private static final Schema schema = + new Schema(Types.StructType.of(required(1, "id", Types.LongType.get())).fields()); + private static final PartitionSpec partitionSpec = builderFor(schema).identity("id").build(); + static final TableIdentifier TABLE_IDENTIFIER = TableIdentifier.of(DB_NAME, TABLE_NAME); + + @RegisterExtension + private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = + new HiveMetastoreExtension( + DB_NAME, ImmutableMap.of(HiveConf.ConfVars.HIVE_TXN_TIMEOUT.varname, "1s")); + + private static HiveCatalog catalog; + private Path tableLocation; + @BeforeAll - public static void startMetastore() throws Exception { - HiveMetastoreTest.startMetastore( - ImmutableMap.of(HiveConf.ConfVars.HIVE_TXN_TIMEOUT.varname, "1s")); + public static void initCatalog() throws Exception { + catalog = + (HiveCatalog) + CatalogUtil.loadCatalog( + HiveCatalog.class.getName(), + CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE, + ImmutableMap.of( + CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, + String.valueOf(TimeUnit.SECONDS.toMillis(10))), + HIVE_METASTORE_EXTENSION.hiveConf()); // start spies - overriddenHiveConf = new Configuration(hiveConf); + overriddenHiveConf = new Configuration(HIVE_METASTORE_EXTENSION.hiveConf()); overriddenHiveConf.setLong("iceberg.hive.lock-timeout-ms", 6 * 1000); overriddenHiveConf.setLong("iceberg.hive.lock-check-min-wait-ms", 50); overriddenHiveConf.setLong("iceberg.hive.lock-check-max-wait-ms", 5 * 1000); @@ -107,14 +139,16 @@ public static void startMetastore() throws Exception { .thenAnswer( invocation -> { // cannot spy on RetryingHiveMetastoreClient as it is a proxy - IMetaStoreClient client = spy(new HiveMetaStoreClient(hiveConf)); + IMetaStoreClient client = + spy(new HiveMetaStoreClient(HIVE_METASTORE_EXTENSION.hiveConf())); spyClientRef.set(client); return spyClientRef.get(); }); spyClientPool.run(IMetaStoreClient::isLocalMetaStore); // To ensure new client is created. - spyCachedClientPool = spy(new CachedClientPool(hiveConf, Collections.emptyMap())); + spyCachedClientPool = + spy(new CachedClientPool(HIVE_METASTORE_EXTENSION.hiveConf(), Collections.emptyMap())); when(spyCachedClientPool.clientPool()).thenAnswer(invocation -> spyClientPool); assertThat(spyClientRef.get()).isNotNull(); @@ -124,6 +158,8 @@ public static void startMetastore() throws Exception { @BeforeEach public void before() throws Exception { + this.tableLocation = + new Path(catalog.createTable(TABLE_IDENTIFIER, schema, partitionSpec).location()); Table table = catalog.loadTable(TABLE_IDENTIFIER); ops = (HiveTableOperations) ((HasTableOperations) table).operations(); String dbName = TABLE_IDENTIFIER.namespace().level(0); @@ -151,6 +187,13 @@ public void before() throws Exception { reset(spyClient); } + @AfterEach + public void dropTestTable() throws Exception { + // drop the table data + tableLocation.getFileSystem(HIVE_METASTORE_EXTENSION.hiveConf()).delete(tableLocation, true); + catalog.dropTable(TABLE_IDENTIFIER, false /* metadata only, location was already deleted */); + } + @AfterAll public static void cleanup() { try {