You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2023/06/07 05:17:33 UTC
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20020
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
IMPALA-12189: updateCatalog should handle failures in createTblTransaction
updateCatalog() invokes createTblTransaction() for transactional tables.
It's called after acquiring the table lock. The write lock of catalog's
versionLock will also be acquired by the current thread. Whenever we hit
an exception, we should release those locks. This patch moves the code
calling createTblTransaction() into the exception handling scope.
Tests:
- Manually modify the code to throw an TransactionException before
calling createTblTransaction. Verified catalogd is hanging after an
INSERT. Verified the issue is fixed by this patch.
Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 14 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/20020/1
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13218/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 08:40:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 10:49:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13217/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 05:39:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20020/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/20020/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6628
PS1, Line 6628: TblTransaction tblTxn = null;
This is an improvement compared to the previous situation, but I think that it would be even better if we would hold less locks while doing this RPC.
Moving the TblTransaction block after catalog_.getLock().writeLock().unlock() in line 6644 would avoid holding a global lock while doing an HMS RPC and seems safe as we don't need tblTxn before that point.
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 08:19:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 3:
(1 comment)
Thanks for the quick review, Csaba!
http://gerrit.cloudera.org:8080/#/c/20020/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/20020/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@6628
PS1, Line 6628: // Get new catalog version for table in insert.
> This is an improvement compared to the previous situation, but I think that
Good point!
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 09:41:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20020
to look at the new patch set (#4).
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
IMPALA-12189: updateCatalog should handle failures in createTblTransaction
updateCatalog() invokes createTblTransaction() for transactional tables.
It's called after acquiring the table lock. The write lock of catalog's
versionLock will also be acquired by the current thread. Whenever we hit
an exception, we should release those locks. This patch moves the code
calling createTblTransaction() into the exception handling scope.
Tests:
- Add a debug action to abort the transaction in updateCatalog() so
createTblTransaction() will fail.
- Add e2e test for the error handling.
Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_acid.py
3 files changed, 53 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/20020/4
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20020
to look at the new patch set (#2).
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
IMPALA-12189: updateCatalog should handle failures in createTblTransaction
updateCatalog() invokes createTblTransaction() for transactional tables.
It's called after acquiring the table lock. The write lock of catalog's
versionLock will also be acquired by the current thread. Whenever we hit
an exception, we should release those locks. This patch moves the code
calling createTblTransaction() into the exception handling scope.
Tests:
- Add a debug action to abort the transaction in updateCatalog() so
createTblTransaction() will fail.
- Add e2e test for the error handling.
Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_acid.py
3 files changed, 52 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/20020/2
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 16:17:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13220/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 10:04:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20020
to look at the new patch set (#3).
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
IMPALA-12189: updateCatalog should handle failures in createTblTransaction
updateCatalog() invokes createTblTransaction() for transactional tables.
It's called after acquiring the table lock. The write lock of catalog's
versionLock will also be acquired by the current thread. Whenever we hit
an exception, we should release those locks. This patch moves the code
calling createTblTransaction() into the exception handling scope.
Tests:
- Add a debug action to abort the transaction in updateCatalog() so
createTblTransaction() will fail.
- Add e2e test for the error handling.
Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_acid.py
3 files changed, 53 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/20020/3
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13219/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 10:02:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9380/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 10:57:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12189: updateCatalog should handle failures in createTblTransaction
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20020 )
Change subject: IMPALA-12189: updateCatalog should handle failures in createTblTransaction
......................................................................
IMPALA-12189: updateCatalog should handle failures in createTblTransaction
updateCatalog() invokes createTblTransaction() for transactional tables.
It's called after acquiring the table lock. The write lock of catalog's
versionLock will also be acquired by the current thread. Whenever we hit
an exception, we should release those locks. This patch moves the code
calling createTblTransaction() into the exception handling scope.
Tests:
- Add a debug action to abort the transaction in updateCatalog() so
createTblTransaction() will fail.
- Add e2e test for the error handling.
Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Reviewed-on: http://gerrit.cloudera.org:8080/20020
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_acid.py
3 files changed, 53 insertions(+), 14 deletions(-)
Approvals:
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/20020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3a64764d0568fc1e6c6f4c52f9e220df3130bd84
Gerrit-Change-Number: 20020
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>