Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/23804 )
Change subject: IMPALA-13863: Add metric to track number of loaded tables in catalogd ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/23804/3/fe/src/main/java/org/apache/impala/catalog/Db.java File fe/src/main/java/org/apache/impala/catalog/Db.java: http://gerrit.cloudera.org:8080/#/c/23804/3/fe/src/main/java/org/apache/impala/catalog/Db.java@218 PS3, Line 218: * or replaced, allowing the caller to track table state transitions. > Done, I removed the methods that only take the param tablename and changed Sorry that if I didn't make this clear. What I suggested is changing the method to be public void addTable(Table table) { tableCache_.add(table, Catalog.TABLE_ADD_UPDATE_FUNC); } We need to make the update functions and the counter static in Catalog.java. Then we don't need to change all the callsites of addTable() and removeTable(). http://gerrit.cloudera.org:8080/#/c/23804/1/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/23804/1/tests/custom_cluster/test_automatic_invalidation.py@125 PS1, Line 125: cursor.execute(self.query) > My bad, I meant sleeps allow async operations in catalogd to complete like If cursor.execute() runs the DDL asynchronorously, we can use self.execute_query(self.query) which will be blocked until the statement finishes and the metrics should have been updated at that point. Then we don't need these sleeps. http://gerrit.cloudera.org:8080/#/c/23804/4/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/23804/4/tests/custom_cluster/test_automatic_invalidation.py@228 PS4, Line 228: # Wait for event processing to complete : time.sleep(self.timeout) Use EventProcessorUtils.wait_for_event_processing(self). http://gerrit.cloudera.org:8080/#/c/23804/4/tests/custom_cluster/test_automatic_invalidation.py@271 PS4, Line 271: # Additional sleep to ensure counter update completes after invalidation : time.sleep(2) I'm confused why we need this sleep. If the table got invalidated, what adds the delay to update the counter? -- To view, visit http://gerrit.cloudera.org:8080/23804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5aa54f9f7507709b654df22e24592799811e8b6c Gerrit-Change-Number: 23804 Gerrit-PatchSet: 4 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 29 Dec 2025 01:54:41 +0000 Gerrit-HasComments: Yes
