flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-4074) Reporter can block TaskManager shutdown
Date Thu, 23 Jun 2016 09:14:16 GMT

    [ https://issues.apache.org/jira/browse/FLINK-4074?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15346125#comment-15346125
] 

ASF GitHub Bot commented on FLINK-4074:
---------------------------------------

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.


> Reporter can block TaskManager shutdown
> ---------------------------------------
>
>                 Key: FLINK-4074
>                 URL: https://issues.apache.org/jira/browse/FLINK-4074
>             Project: Flink
>          Issue Type: Improvement
>          Components: Metrics
>    Affects Versions: 1.1.0
>            Reporter: Chesnay Schepler
>            Assignee: Chesnay Schepler
>            Priority: Minor
>             Fix For: 1.1.0
>
>
> If a report is being submitted while a TaskManager is shutting down the reporter can
cause the shutdown to be delayed since it submits the complete report.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message