You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/06/04 21:27:42 UTC
[GitHub] [hbase] ndimiduk commented on a change in pull request #1749: HBASE-24367 ScheduledChore log elapsed timespan in a human-friendly format
ndimiduk commented on a change in pull request #1749:
URL: https://github.com/apache/hbase/pull/1749#discussion_r435562854
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
##########
@@ -183,18 +176,21 @@ public void run() {
if (LOG.isInfoEnabled()) LOG.info("Chore: " + getName() + " was stopped");
} else {
try {
+ // TODO: Histogram metrics per chore name.
+ // For now, just measure and log if DEBUG level logging is enabled.
+ long start = 0;
+ if (LOG.isDebugEnabled()) {
+ start = System.nanoTime();
+ }
if (!initialChoreComplete) {
initialChoreComplete = initialChore();
} else {
- timeMeasurement.measure(() -> {
chore();
- return null;
- });
- if (LOG.isInfoEnabled() && (System.nanoTime() - lastLog > FIVE_MINUTES_IN_NANOS)) {
- LOG.info("{} average execution time: {} ns.", getName(),
- (long)(timeMeasurement.getAverageTime()));
- lastLog = System.nanoTime();
- }
+ }
+ if (LOG.isDebugEnabled()) {
+ long end = System.nanoTime();
+ LOG.debug("{} execution time: {} ms.", getName(),
Review comment:
I was expecting to find something like `if (LOG.isDebugEnabled() && start > 0) {` here. Was this comment addressed elsewhere? Thanks.
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
##########
@@ -183,18 +176,21 @@ public void run() {
if (LOG.isInfoEnabled()) LOG.info("Chore: " + getName() + " was stopped");
} else {
try {
+ // TODO: Histogram metrics per chore name.
+ // For now, just measure and log if DEBUG level logging is enabled.
+ long start = 0;
+ if (LOG.isDebugEnabled()) {
+ start = System.nanoTime();
+ }
if (!initialChoreComplete) {
initialChoreComplete = initialChore();
} else {
- timeMeasurement.measure(() -> {
chore();
- return null;
- });
- if (LOG.isInfoEnabled() && (System.nanoTime() - lastLog > FIVE_MINUTES_IN_NANOS)) {
Review comment:
yeah i think you're right. let's leave it for now.
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java
##########
@@ -183,18 +176,21 @@ public void run() {
if (LOG.isInfoEnabled()) LOG.info("Chore: " + getName() + " was stopped");
} else {
try {
+ // TODO: Histogram metrics per chore name.
+ // For now, just measure and log if DEBUG level logging is enabled.
+ long start = 0;
+ if (LOG.isDebugEnabled()) {
+ start = System.nanoTime();
Review comment:
I'm not asking for a scope change, just clarifying my understanding.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org