Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles ...................................................................... Patch Set 3: Code-Review+1 (7 comments) http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java File fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java@46 PS2, Line 46: public void setLastLoadedTimeMs(long timeMs) { > Yes, we need it in ImpaladCatalog. When impalad applies the update from cat thanks for the explanation! Done http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java File fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java@70 PS2, Line 70: > Good point! Done. Done http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java File fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java@66 PS2, Line 66: > Done Done http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@346 PS2, Line 346: catalogObject.getCatalog_version(), catalogObject.getLast_modified_time_ms()); > They are currently not used in impalad yet, e.g. not shown in the query pro Done http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@807 PS2, Line 807: resp.object_loaded_time_ms, new SqlConstraints(primaryKeys, foreignKeys), > Same as above, just set it for tables to be simple. Done http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2584 PS2, Line 2584: > Done Done http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585 PS2, Line 2585: // Add the catalog versions and loaded timestamps. > I hope it can be parsed by tools like Workload Management. Maybe the curren I'm not sure about the Thrift/JSON profile. If we cannot use different formats, it's definitely better to be parseable in Thrift/JSON. Workload management reads from the underlying ClientRequestState object that was built for each query. As long as this information is accessible there (easy to add if not), then it can be read by workload management. -- To view, visit http://gerrit.cloudera.org:8080/21782 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98 Gerrit-Change-Number: 21782 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Thu, 12 Sep 2024 19:15:21 +0000 Gerrit-HasComments: Yes
