Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14038 )
Change subject: IMPALA-8823 WIP: DROP TABLE support for insert-only ACID tables ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/14038/5/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14038/5/fe/src/main/java/org/apache/impala/service/Frontend.java@510 PS5, Line 510: table Can this table be instanceof Incomplete table in case there is a tableloading exception during analysis? http://gerrit.cloudera.org:8080/#/c/14038/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1721 PS5, Line 1721: transactionKeepalive_.deleteLock(lockId); : MetastoreShim.releaseLock(client.getHiveClient(), lockId); Is it possible that we are silently leaking locks if for instance MetastoreShim.releaseLock throws an exception (say due to connection issue with HMS)? According to me both these statements should happen atomically. Either they both succeed or not. http://gerrit.cloudera.org:8080/#/c/14038/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1809 PS5, Line 1809: lockId = MetastoreShim.acquireLock(client.getHiveClient(), 0L, lockComponents, : LOCK_RETRIES, LOCK_RETRY_WAIT_SECONDS); : transactionKeepalive_.addLock(lockId, queryCtx); same comment as previous regarding the atomicity of these two statemnts. May be provide this as a API in the transactionKeepAlive so that we can reuse that code elsewhere if needed. -- To view, visit http://gerrit.cloudera.org:8080/14038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic41ca73268c4b75af5a08fe3dd1ada1df3f6fd34 Gerrit-Change-Number: 14038 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 09 Aug 2019 21:14:32 +0000 Gerrit-HasComments: Yes