This is an automated email from the ASF dual-hosted git repository.
pvary pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 9114cc87ac Hive: Use base table metadata to create HiveLock (#10016)
9114cc87ac is described below
commit 9114cc87ac48ac77402eb41d398c6371a79f3c79
Author: Rui Li <[email protected]>
AuthorDate: Fri May 24 03:23:24 2024 +0800
Hive: Use base table metadata to create HiveLock (#10016)
---
.../apache/iceberg/hive/HiveTableOperations.java | 6 ++--
.../org/apache/iceberg/hive/TestHiveCommits.java | 39 ++++++++++++++++++++--
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git
a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
index 5293f91540..64f0913852 100644
---
a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
+++
b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
@@ -179,7 +179,7 @@ public class HiveTableOperations extends
BaseMetastoreTableOperations
BaseMetastoreOperations.CommitStatus.FAILURE;
boolean updateHiveTable = false;
- HiveLock lock = lockObject(metadata);
+ HiveLock lock = lockObject(base);
try {
lock.lock();
@@ -242,7 +242,7 @@ public class HiveTableOperations extends
BaseMetastoreTableOperations
try {
persistTable(
- tbl, updateHiveTable, hiveLockEnabled(metadata, conf) ? null :
baseMetadataLocation);
+ tbl, updateHiveTable, hiveLockEnabled(base, conf) ? null :
baseMetadataLocation);
lock.ensureActive();
commitStatus = BaseMetastoreOperations.CommitStatus.SUCCESS;
@@ -510,7 +510,7 @@ public class HiveTableOperations extends
BaseMetastoreTableOperations
* @return if the hive engine related values should be enabled or not
*/
private static boolean hiveLockEnabled(TableMetadata metadata, Configuration
conf) {
- if (metadata.properties().get(TableProperties.HIVE_LOCK_ENABLED) != null) {
+ if (metadata != null &&
metadata.properties().get(TableProperties.HIVE_LOCK_ENABLED) != null) {
// We know that the property is set, so default value will not be used,
return metadata.propertyAsBoolean(TableProperties.HIVE_LOCK_ENABLED,
false);
}
diff --git
a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java
b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java
index acf4f8dc5c..b3bbde4606 100644
--- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java
+++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java
@@ -18,6 +18,7 @@
*/
package org.apache.iceberg.hive;
+import static org.apache.iceberg.TableProperties.HIVE_LOCK_ENABLED;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.any;
@@ -39,6 +40,7 @@ import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.CommitStateUnknownException;
import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.types.Types;
import org.apache.thrift.TException;
import org.junit.jupiter.api.Test;
@@ -64,7 +66,7 @@ public class TestHiveCommits extends HiveTableBaseTest {
AtomicReference<HiveLock> lockRef = new AtomicReference<>();
- when(spyOps.lockObject(metadataV1))
+ when(spyOps.lockObject(metadataV2))
.thenAnswer(
i -> {
HiveLock lock = (HiveLock) i.callRealMethod();
@@ -273,11 +275,11 @@ public class TestHiveCommits extends HiveTableBaseTest {
AtomicReference<HiveLock> lock = new AtomicReference<>();
doAnswer(
l -> {
- lock.set(ops.lockObject(metadataV1));
+ lock.set(ops.lockObject(metadataV2));
return lock.get();
})
.when(spyOps)
- .lockObject(metadataV1);
+ .lockObject(metadataV2);
concurrentCommitAndThrowException(ops, spyOps, table, lock);
@@ -415,6 +417,37 @@ public class TestHiveCommits extends HiveTableBaseTest {
.hasMessageStartingWith("null\nCannot determine whether the commit was
successful or not");
}
+ @Test
+ public void testChangeLockWithAlterTable() throws Exception {
+ Table table = catalog.loadTable(TABLE_IDENTIFIER);
+ HiveTableOperations ops = (HiveTableOperations) ((HasTableOperations)
table).operations();
+ TableMetadata base = ops.current();
+ final HiveLock initialLock = ops.lockObject(base);
+
+ AtomicReference<HiveLock> lockRef = new AtomicReference<>();
+ HiveTableOperations spyOps = spy(ops);
+ doAnswer(
+ i -> {
+ lockRef.set(ops.lockObject(i.getArgument(0)));
+ return lockRef.get();
+ })
+ .when(spyOps)
+ .lockObject(base);
+
+ TableMetadata newMetadata =
+ TableMetadata.buildFrom(base)
+ .setProperties(
+ ImmutableMap.of(
+ HIVE_LOCK_ENABLED, initialLock instanceof NoLock ? "true"
: "false"))
+ .build();
+ spyOps.commit(base, newMetadata);
+
+ assertThat(lockRef).as("Lock not captured by the
stub").doesNotHaveNullValue();
+ assertThat(lockRef.get())
+ .as("New lock mechanism shouldn't take effect before the commit
completes")
+ .hasSameClassAs(initialLock);
+ }
+
private void commitAndThrowException(
HiveTableOperations realOperations, HiveTableOperations spyOperations)
throws TException, InterruptedException {
