Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8545 )
Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@596 PS1, Line 596: addTableToCatalogDeltaHelper(tbl, fromVersion, toVersion, resp); Maybe I'm misunderstanding something but I still see the lock contention on 'tbl' here. With the patch, given we don't lock the tables while snapshotting, there could be inflight DDLs on tbl stuck in tryLockTable() (say CatalogOpEx.alterTable()) and that still holds the tbl.lock_ and still hasn't updated tbl to a newer catalog version resulting in the if check in L595 to pass. Eventually, that blocks on L622 stalling the getCatalogDelta() thread. For example, we are trying to snapshot objects in (90, 100] and tbl's version is 95. We have an alterTable() running on this particular table which got the new catalog version (say 105), but hasn't still set it for the 'tbl' since it is waiting for the alter() to finish. All this while, alter thread still holds the tbl.lock_, but the condition in L595 still passes with this thread blocking on L622. (My argument is based on my understanding is that 'tbl' still refers to the same table object even after ImmutableList.copy() in L564) Did I get something wrong? -- To view, visit http://gerrit.cloudera.org:8080/8545 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3 Gerrit-Change-Number: 8545 Gerrit-PatchSet: 1 Gerrit-Owner: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Comment-Date: Thu, 16 Nov 2017 06:11:31 +0000 Gerrit-HasComments: Yes
