Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 )
Change subject: IMPALA-12540: Query Live Table ...................................................................... Patch Set 41: (5 comments) I'll work on the other comments. http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h File be/src/exec/system-table-scan-node.h: http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h@43 PS41, Line 43: NYI > nit: Not Yet Implemented? Yeah, this pattern appears to be fairly common across PlanNode implementations. See also data-source-scan-node.h and hbase-scan-node.h. http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@672 PS41, Line 672: QUERY_LIVE > Since QUERY_LIVE name is constant, should this be a const instead? Well, I do use it to constrain the options in TSystemTable, and it reduces data needing to be serialized. http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java File fe/src/main/java/org/apache/impala/catalog/SystemTable.java: http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@176 PS41, Line 176: : private static org.apache.hadoop.hive.metastore.api.Table : createMetastoreTable(String dbName, String tableName, String owner) { > This is not registering the table to HMS, isn't it? Right, it doesn't send anything to HMS. This is an Impala-only table, that can only be satisfied by coordinators, so not sure what anyone would do with it if it were in HMS. http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java: http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@121 PS41, Line 121: 1024L * 1024L > SQL, PLAN, and EXEC_SUMMARY can be long. Has this take account for that? No, honestly no idea what to put here. I guess this is another area we could estimate from current active queries. http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@27 PS41, Line 27: @CustomClusterTestSuite.with_args(impalad_args="--enable_workload_mgmt " : "--cluster_id=test_query_live_1", : catalogd_args="--enable_workload_mgmt") > Can this and the other test work without workload management enabled? sys.impala_query_live is currently only accessible if workload management is enabled. That's flagged in CatalogServiceCatalog.java, CatalogBlacklistUtils.java, and Db.java. -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 41 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 20 Mar 2024 18:08:27 +0000 Gerrit-HasComments: Yes
