flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tillrohrmann <...@git.apache.org>
Subject [GitHub] flink pull request #2105: [FLINK-4074] Make metric reporters less blocking
Date Thu, 23 Jun 2016 09:13:52 GMT
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2105#discussion_r68200885
  
    --- Diff: flink-core/src/main/java/org/apache/flink/metrics/MetricRegistry.java ---
    @@ -118,40 +120,38 @@ public MetricRegistry(Configuration config) {
     
     				if (reporter instanceof Scheduled) {
     					LOG.info("Periodically reporting metrics in intervals of {} {}", period, timeunit.name());
    -					long millis = timeunit.toMillis(period);
     					
    -					timer = new java.util.Timer("Periodic Metrics Reporter", true);
    -					timer.schedule(new ReporterTask((Scheduled) reporter), millis, millis);
    +					executor.scheduleWithFixedDelay(new ReporterTask((Scheduled) reporter), period,
period, timeunit);
     				}
     				else {
    -					timer = null;
    +					executor = null;
     				}
     			}
     			catch (Throwable t) {
     				reporter = new JMXReporter();
    -				timer = null;
    +				executor = null;
     				LOG.error("Could not instantiate custom metrics reporter. Defaulting to JMX metrics
export.", t);
     			}
     
     			this.reporter = reporter;
    -			this.timer = timer;
    +			this.executor = executor;
     		}
     	}
     
     	/**
     	 * Shuts down this registry and the associated {@link org.apache.flink.metrics.reporter.MetricReporter}.
     	 */
     	public void shutdown() {
    -		if (timer != null) {
    -			timer.cancel();
    -		}
     		if (reporter != null) {
     			try {
     				reporter.close();
     			} catch (Throwable t) {
     				LOG.warn("Metrics reporter did not shut down cleanly", t);
     			}
     		}
    +		if (executor != null) {
    +			executor.shutdownNow();
    --- End diff --
    
    I guess that one second of delay wouldn't really hurt. That's at least how the `ScheduledReporter`
do it.
    
    Btw: Why do we actually replicate the execution logic for the `com.codahale.metrics.ScheduledReporter`.
As far as I can tell, we could simply use the internal reporting mechanism. Then we would
not have to implement the `Scheduled` interface.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message