Sai Hemanth Gantasala has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20473 )

Change subject: IMPALA-11553: Add event specific metrics in the table metrics
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20473/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20473/2//COMMIT_MSG@22
PS2, Line 22:
> Can we add more metrics at table level, e.g. events-skipped, last-synced-ev
last-synced-event-id on table metric makes sense to me.
I don't think adding "events-skipped", "events-consuming-delay-ms" on table 
metrics adds much value. These metrics on event-processor summary page makes 
much better sense to me. Let me know if see good motivation to add them on 
table metrics.


http://gerrit.cloudera.org:8080/#/c/20473/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File 
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:

http://gerrit.cloudera.org:8080/#/c/20473/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@942
PS2, Line 942:       Preconditions.checkState(event.getEventId() == 
currentEventId);
> I made a mistake that the unit is actually second, not milli second. Could
Ack


http://gerrit.cloudera.org:8080/#/c/20473/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1063
PS2, Line 1063:
> TimeUnit.SECONDS ?
Ack


http://gerrit.cloudera.org:8080/#/c/20473/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1078
PS2, Line 1078:       LOG.info("Time elapsed in processing event batch: {}",
> We use 'System.currentTimeMillis() / 1000' directly in many places. Maybe d
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2428029361e610a0fcd8ed11be2ab771f03b00dd
Gerrit-Change-Number: 20473
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Fri, 15 Sep 2023 00:27:37 +0000
Gerrit-HasComments: Yes

Reply via email to