Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8529 )
Change subject: [PREVIEW] IMPALA-4886: Expose table metrics in the catalog web UI. ...................................................................... Patch Set 1: (3 comments) I took a very brief look. I was worried when you said "lightweight framework", but it turns out you're using codahale/yammer metrics, which is the right thing to do. You know this, but TopNCache doesn't have a unit test. I didn't look at all of this. http://gerrit.cloudera.org:8080/#/c/8529/1/common/thrift/JniCatalog.thrift File common/thrift/JniCatalog.thrift: http://gerrit.cloudera.org:8080/#/c/8529/1/common/thrift/JniCatalog.thrift@602 PS1, Line 602: 1: required list<string> large_tables Is the idea that len(large_tables)==len(memory_estimates), and likewise len(frequent_table)=len(num_metadata_operations)? I think it'd be more honest for this to be "list<LargeTable> large_tables", with LargeTable being a struct that has a tablename and details about that table that you want to share. http://gerrit.cloudera.org:8080/#/c/8529/1/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/8529/1/fe/pom.xml@365 PS1, Line 365: <artifactId>metrics-core</artifactId> Thanks. This is the right thing to use in my experience. http://gerrit.cloudera.org:8080/#/c/8529/1/fe/src/main/java/org/apache/impala/catalog/CatalogUsageMonitor.java File fe/src/main/java/org/apache/impala/catalog/CatalogUsageMonitor.java: http://gerrit.cloudera.org:8080/#/c/8529/1/fe/src/main/java/org/apache/impala/catalog/CatalogUsageMonitor.java@36 PS1, Line 36: // TODO: Consider making it a configurable parameter. A somewhat cheap way to do this is to use: Integer.getInteger("org.apache.impala.catalog.CatalogUsageMonitor.NUM_TABLES_TRACKED", 25) here. That gets the number from system properties. It's pretty weak configuration, but I've used it to nice effect for constants that really should be fine as is. -- To view, visit http://gerrit.cloudera.org:8080/8529 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e Gerrit-Change-Number: 8529 Gerrit-PatchSet: 1 Gerrit-Owner: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Tue, 14 Nov 2017 20:00:54 +0000 Gerrit-HasComments: Yes
