Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22930 )
Change subject: IMPALA-6406: Add CatalogActionLocker for fine grained DDL locking ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/22930/5/fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java File fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java: http://gerrit.cloudera.org:8080/#/c/22930/5/fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java@125 PS5, Line 125: final Map<ResourceKey, Action> dbActions = new HashMap<>(); : final Map<ResourceKey, Action> tableActions = new HashMap<>(); We need to make sure no leaks in these maps, i.e. if a db/table no longer exists in catalog, the corresponding key-value pair should also be removed. Haven't looked into the details but I have two questions: - When a db is dropped, will all the key-value pairs of its underlying tables be dropped as well? - When creating a db/table fails in HMS side, will the corresponding key-value pair be removed? http://gerrit.cloudera.org:8080/#/c/22930/5/fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java@147 PS5, Line 147: NUM_PARTITIONS We need this to be configurable. http://gerrit.cloudera.org:8080/#/c/22930/5/fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java@261 PS5, Line 261: try { We'd better add a thread annotation (ThreadNameAnnotator) for this so jstacks can show what the thread is waiting for. http://gerrit.cloudera.org:8080/#/c/22930/5/fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java@273 PS5, Line 273: action = actionMap.get(key); Why do we need to get it again here? Will other threads replace it? http://gerrit.cloudera.org:8080/#/c/22930/5/fe/src/main/java/org/apache/impala/catalog/CatalogActionLocker.java@392 PS5, Line 392: e instanceof CatalogException e is always CatalogException (declared on L384). -- To view, visit http://gerrit.cloudera.org:8080/22930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2ed48d92333787f75b64b8bd878834612433f6f8 Gerrit-Change-Number: 22930 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu <wydbaggio...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com> Gerrit-Comment-Date: Thu, 19 Jun 2025 13:52:31 +0000 Gerrit-HasComments: Yes
