[Impala-ASF-CR] IMPALA-6663 Expose current DDL metrics on WebUI

["Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>]

Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13806 )

Change subject: IMPALA-6663 Expose current DDL metrics on WebUI
......................................................................


Patch Set 7:

(4 comments)

I think this in general is a very useful addition. Can you also specify the motivation of why we need to track the total number of DDLs instead of tracking them on a per table level. Certain metadata operations like invalidate metadata as indeed global and cannot be tracked at table. However, I think it useful to track how many refresh or invalidates were called on a given table and finding top-n such tables. Similarly for DMLs, instead of a getting a total count of INSERTS, would it be more useful to get top-N tables which are inserted into?

http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java
File fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java:

http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java@41
PS7, Line 41:   private AtomicLong resetMetadataCounter_;
            : 
            :   private AtomicLong dmlOperationCounter_;
            : 
            :   private AtomicLong syncDdlOperationCounter_;
Can you add a one line comment on each of these counters as to what do they represent? Also, would be great if you could specify what operations constitute a DML in this context.


http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java@70
PS7, Line 70: decrementDdlTypeCounter
Is decrement ever needed? same for the other counters?


http://gerrit.cloudera.org:8080/#/c/13806/7/fe/src/main/java/org/apache/impala/catalog/monitor/CatalogOpRequestCounter.java@109
PS7, Line 109: getResetMetadataCounter
By returning the AtomicLong object this class is leaking the private field and it is subject to modification by other classes. A more common pattern is to return its value (return type long).


http://gerrit.cloudera.org:8080/#/c/13806/7/www/catalog.tmpl
File www/catalog.tmpl:

http://gerrit.cloudera.org:8080/#/c/13806/7/www/catalog.tmpl@202
PS7, Line 202: Running
Is this user-visible text? Running seems to suggest these operations are currently being run.



-- 
To view, visit http://gerrit.cloudera.org:8080/13806
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0ed76f134bad6d3b3d4dce132365a53a01e9512a
Gerrit-Change-Number: 13806
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 17:24:38 +0000
Gerrit-HasComments: Yes