impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] [PREVIEW] IMPALA-4886: Expose table metrics in the catalog web UI.
Date Tue, 14 Nov 2017 20:00:54 GMT
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 <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Comment-Date: Tue, 14 Nov 2017 20:00:54 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message