Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/23811 )
Change subject: IMPALA-14641: Add detailed operation page for Catalogd operations ...................................................................... Patch Set 13: (3 comments) http://gerrit.cloudera.org:8080/#/c/23811/13/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java File fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java: http://gerrit.cloudera.org:8080/#/c/23811/13/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOperationTracker.java@92 PS13, Line 92: if (queryId_ == null) { : return key.queryId_ == null && threadId_ == key.threadId_; : } I think we can remove these null branches since all the callsites on the constructor ensures queryId is not null. E.g. addRecord() returns fast when queryId is null. http://gerrit.cloudera.org:8080/#/c/23811/13/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/23811/13/tests/webserver/test_web_pages.py@1274 PS13, Line 1274: assert "response_size_bytes" in detail_data, "response_size_bytes field missing" : : # Verify the fields contain actual data : assert len(detail_data["timeline"]) > 0, "timeline should not be empty" : assert detail_data["request_size_bytes"] > 0, "request_size_bytes should be positive" : assert detail_data["response_size_bytes"] > 0, \ : "response_size_bytes should be positive" : : # Verify timeline is formatted text (pre-formatted on server side) : timeline_str = detail_data["timeline"] : assert isinstance(timeline_str, str), "timeline should be a formatted string" : : # Check for expected timeline format from RuntimeProfile : assert "Catalog Server Operation:" in timeline_str, \ : "timeline should have the operation name" : # Check that it contains typical timeline events : assert ("Got" in timeline_str or "DDL" in timeline_str : or "finished" in timeline_str), "timeline should contain at least one event" Wrong indentation here. http://gerrit.cloudera.org:8080/#/c/23811/13/tests/webserver/test_web_pages.py@1293 PS13, Line 1293: LOG.info("Operation detail test passed - all fields verified") nit: Remove useless logging. The logs are only visible when the test fails. -- To view, visit http://gerrit.cloudera.org:8080/23811 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib80ce3b5873672065b04b00a21d3419a1db0969c Gerrit-Change-Number: 23811 Gerrit-PatchSet: 13 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Mon, 23 Mar 2026 11:28:35 +0000 Gerrit-HasComments: Yes
