You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2023/01/10 11:51:51 UTC
[GitHub] [hive] kasakrisz opened a new pull request, #3934: Hive 26922 master mv iceberg rebuild deadlock
kasakrisz opened a new pull request, #3934:
URL: https://github.com/apache/hive/pull/3934
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
Set the lock type based in the table's storage handler in case of non native tables when creating LockComponent's for output tables.
### Why are the changes needed?
Iceberg table writes does not require exclusive lock since Iceberg handles the concurrency. However locking is necessary when committing the Iceberg change and updating the metadata file at Hive side. Originally for insert overwrite an exclusive lock was acquired in the beginning of the transaction to the target table and a second write lock was requested for the Iceberg commit in the same transaction but this second one was never been acquired because the two lock request caused a deadlock.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
```
mvn test -Dtest.output.overwrite -Dtest=TestIcebergCliDriver -Dqfile=mv_iceberg_orc2.q -pl itests/qtest-iceberg -Piceberg -Pitests -Drat.skip
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3934:
URL: https://github.com/apache/hive/pull/3934#issuecomment-1378380572
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3934)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3934:
URL: https://github.com/apache/hive/pull/3934#issuecomment-1380163168
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3934)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3934:
URL: https://github.com/apache/hive/pull/3934#issuecomment-1380829218
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3934)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1067803435
##########
iceberg/iceberg-handler/src/test/queries/positive/mv_iceberg_orc2.q:
##########
@@ -22,3 +22,12 @@ select * from mat1;
explain cbo
select tbl_ice.b, tbl_ice.c from tbl_ice where tbl_ice.c > 52;
+
+insert into tbl_ice values (10, 'ten', 60);
Review Comment:
added test for v2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1073566847
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output, Table t) {
+ final HiveStorageHandler storageHandler = Preconditions.checkNotNull(t.getStorageHandler(),
+ "Non-native tables must have an instance of storage handler.");
+ LockType lockType = storageHandler.getLockType(output);
+ if (null == LockType.findByValue(lockType.getValue())) {
Review Comment:
I haven't found any benefit :)
I just extracted these lines to a method to reuse this logic in case of insert overwrite.
Removed the findByValue part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] amansinha100 commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
amansinha100 commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1067357268
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output, Table t) {
+ final HiveStorageHandler storageHandler = Preconditions.checkNotNull(t.getStorageHandler(),
+ "Thought all the non native tables have an instance of storage handler");
Review Comment:
'Thought' is misplaced here. Although this was in the original code too, this should be rephrased..something like 'Non native tables should have an instance of storage handler'.
##########
iceberg/iceberg-handler/src/test/queries/positive/mv_iceberg_orc2.q:
##########
@@ -22,3 +22,12 @@ select * from mat1;
explain cbo
select tbl_ice.b, tbl_ice.c from tbl_ice where tbl_ice.c > 52;
+
+insert into tbl_ice values (10, 'ten', 60);
Review Comment:
In addition to the INSERT test, can we add one for DELETE as well ? This would need format-version=2 for the source table. Also, I assume v2 is supported for the MV itself so having a sanity test where the MV is created with format-version=2 would be useful to check no deadlocks occur in that case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz merged pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz merged PR #3934:
URL: https://github.com/apache/hive/pull/3934
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1073259594
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output, Table t) {
+ final HiveStorageHandler storageHandler = Preconditions.checkNotNull(t.getStorageHandler(),
+ "Non-native tables must have an instance of storage handler.");
+ LockType lockType = storageHandler.getLockType(output);
+ if (null == LockType.findByValue(lockType.getValue())) {
+ throw new IllegalArgumentException(String
+ .format("Lock type [%s] for Database.Table [%s.%s] is unknown", lockType, t.getDbName(),
Review Comment:
`getCompleteName()` uses `@` as separator char. I think without quoting db and table identifiers both separator chars (`.` and `@`) adds the same value since both of them can be part of the identifier.
In this case this is not an issue because we just add the name to an exception message and the message also contains the format.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on PR #3934:
URL: https://github.com/apache/hive/pull/3934#issuecomment-1387079817
Lock requests in the code:
1. https://github.com/apache/hive/blob/55471330426c2e0a52101c2e535a66f751be76ee/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java#L3029
2. https://github.com/apache/hive/blob/55471330426c2e0a52101c2e535a66f751be76ee/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCommitLock.java#L118
The first lock is acquired only if these setting are present
```
set hive.support.concurrency=true;
set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
```
These are required for native acid operations.
The deadlock happens in a few lines later in the 2. place: after the lock request is issued we check periodically the state of the lock whether it is acquired or not. Unfortunately in this use case it is always in a waiting state since another exclusive write lock already acquired on the same table in the 1. place and that one is released only when the current txn ends.
In this patch my goal was to copy the behavior from the insert case: let the table's storage handler determine the type of lock in the 1. place.
https://github.com/apache/hive/blob/55471330426c2e0a52101c2e535a66f751be76ee/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java#L3068
In case of Iceberg it is `SHARED_READ`. Iceberg uses optimistic concurrency control so multiple transactions can write the same table parallel only the Iceberg commit step is serialized using exclusive write locks.
Currently we support only full rebuild of Iceberg Materialized views which is technically an insert overwrite. So a plain insert overwrite statement is also affected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1073584822
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/StorageHandlerMock.java:
##########
@@ -70,7 +70,7 @@ public class StorageHandlerMock extends DefaultStorageHandler {
if (writeEntity.getWriteType().equals(WriteEntity.WriteType.INSERT)) {
return LockType.SHARED_READ;
}
- return LockType.SHARED_WRITE;
+ return LockType.EXCLUSIVE;
Review Comment:
This mock is used in 2 test cases:
* testLockingOnInsertIntoNonNativeTables
* testLockingOnInsertOverwriteNonNativeTables
Prior this patch this was ignored in case of insert overwrite and `EXCLUSIVE` was set in case of any type of non transactional table.
After I altered the logic to use the lock type coming from the `storageHandler` the mock returned `SHARED_WRITE` and test `testLockingOnInsertOverwriteNonNativeTables` failed since it still expected `EXCLUSIVE`.
I could alter the assertion in the test but since the default lock type specified by the storage handler is `EXCLUSIVE` I chose altering the mock.
https://github.com/apache/hive/blob/55471330426c2e0a52101c2e535a66f751be76ee/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java#L199-L201
Maybe `super.getLockType` would be better here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] scarlin-cloudera commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
scarlin-cloudera commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1072847233
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output, Table t) {
+ final HiveStorageHandler storageHandler = Preconditions.checkNotNull(t.getStorageHandler(),
+ "Non-native tables must have an instance of storage handler.");
+ LockType lockType = storageHandler.getLockType(output);
+ if (null == LockType.findByValue(lockType.getValue())) {
+ throw new IllegalArgumentException(String
+ .format("Lock type [%s] for Database.Table [%s.%s] is unknown", lockType, t.getDbName(),
Review Comment:
Optional nit: I see this was copied from somewhere else, but there really only has to be one argument here, t.getCompleteName()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3934:
URL: https://github.com/apache/hive/pull/3934#issuecomment-1396235662
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3934)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3934:
URL: https://github.com/apache/hive/pull/3934#issuecomment-1377430773
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3934)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3934&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3934&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3934&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kasakrisz commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
kasakrisz commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1067803159
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output, Table t) {
+ final HiveStorageHandler storageHandler = Preconditions.checkNotNull(t.getStorageHandler(),
+ "Thought all the non native tables have an instance of storage handler");
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a diff in pull request #3934: HIVE-26922: Deadlock when rebuilding Materialized view stored by Iceberg
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3934:
URL: https://github.com/apache/hive/pull/3934#discussion_r1073456835
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3122,7 +3117,19 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
}
return lockComponents;
}
-
+
+ private static LockType getLockTypeFromStorageHandler(WriteEntity output, Table t) {
+ final HiveStorageHandler storageHandler = Preconditions.checkNotNull(t.getStorageHandler(),
+ "Non-native tables must have an instance of storage handler.");
+ LockType lockType = storageHandler.getLockType(output);
+ if (null == LockType.findByValue(lockType.getValue())) {
Review Comment:
What's the benefit of doing `findByValue`? Isn't `if(null == lockType)` already sufficient?
##########
ql/src/test/org/apache/hadoop/hive/ql/metadata/StorageHandlerMock.java:
##########
@@ -70,7 +70,7 @@ public class StorageHandlerMock extends DefaultStorageHandler {
if (writeEntity.getWriteType().equals(WriteEntity.WriteType.INSERT)) {
return LockType.SHARED_READ;
}
- return LockType.SHARED_WRITE;
+ return LockType.EXCLUSIVE;
Review Comment:
Changing the lock means that we are changing the tests. Why is it necessary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org