Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15260 )
Change subject: IMPALA-9357: Fix race condition in alter_database event ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/15260/6/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/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@436 PS6, Line 436: * method. > optional: I think this is is not too complex. Maybe we can do it now. We ca I think this approach definitely helps with reducing the duplicate code, but the interface definition is still not very clean. Ideally, a Lockable interface should have methods like lock(), unlock() and tryLock(). This makes sure that lock object itself is a private member of Table and Db and I think thats a better approach. But inorder to do this all the occurances of table.getLock().lock() will need to change and a number of unrelated files will be added to this change. I would like to separate this to a separate patch. Created IMPALA-9468 for this. http://gerrit.cloudera.org:8080/#/c/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@457 PS6, Line 457: uptedException e) > nit: It'd be better to rename this to something not related to table, like renamed to LOCK_RETRY_DELAY_MS http://gerrit.cloudera.org:8080/#/c/15260/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@462 PS6, Line 462: > nit: same here, maybe RETRY_LOCK_TIMEOUT_MS. changed to LOCK_RETRY_TIMEOUT_MS -- To view, visit http://gerrit.cloudera.org:8080/15260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e Gerrit-Change-Number: 15260 Gerrit-PatchSet: 7 Gerrit-Owner: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Anurag Mantripragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Fri, 06 Mar 2020 01:24:06 +0000 Gerrit-HasComments: Yes
