You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2018/02/02 08:46:19 UTC

[GitHub] flink pull request #5402: [FLINK-8549] [config] Move TimerServiceOptions int...

GitHub user StephanEwen opened a pull request:

    https://github.com/apache/flink/pull/5402

    [FLINK-8549] [config] Move TimerServiceOptions into TaskManagerOptions

    The `TimerServiceOptions` are in the wrong place, which prohibits generation of config docs. It also cause over-fragmentation of the options in the code base.
    
    This PR moves the one option from that class to the `TaskManagerOptions`, as it relates to task execution. Other shutdown related options are in there already.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/StephanEwen/incubator-flink timer_service_options

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/5402.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5402
    
----
commit f5045b211ce95a41fe1c6f77fdeacbe57f930c8e
Author: Stephan Ewen <se...@...>
Date:   2018-02-02T08:38:37Z

    [FLINK-8549] [config] Move TimerServiceOptions into TaskManagerOptions

----


---

[GitHub] flink issue #5402: [FLINK-8549] [config] Move TimerServiceOptions into TaskM...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5402
  
    Thanks for the heads up - nice to know that the generator also supports different projects now.
    
    I am still leaning towards making this change, because all other shutdown options are already in the `TaskManagerOptions`, and this class just felt out of place in `flink-streaming-java`.


---

[GitHub] flink pull request #5402: [FLINK-8549] [config] Move TimerServiceOptions int...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5402#discussion_r165626728
  
    --- Diff: flink-core/src/main/java/org/apache/flink/configuration/TaskManagerOptions.java ---
    @@ -206,6 +206,14 @@
     			key("task.cancellation.timeout")
     			.defaultValue(180000L);
     
    +	/**
    +	 * This configures how long we wait for the timers to finish all pending timer threads
    +	 * when the stream task is cancelled .
    +	 */
    +	public static final ConfigOption<Long> TASK_CANCELLATION_TIMEOUT_TIMERS = ConfigOptions
    +			.key("task.cancellation.timeout.timers")
    +			.defaultValue(7500L);
    --- End diff --
    
    Good point - I just checked, it was in fact already part of 1.4 (I had falsely in my mind that it was new in 1.5)


---

[GitHub] flink pull request #5402: [FLINK-8549] [config] Move TimerServiceOptions int...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5402#discussion_r165623427
  
    --- Diff: flink-core/src/main/java/org/apache/flink/configuration/TaskManagerOptions.java ---
    @@ -206,6 +206,14 @@
     			key("task.cancellation.timeout")
     			.defaultValue(180000L);
     
    +	/**
    +	 * This configures how long we wait for the timers to finish all pending timer threads
    +	 * when the stream task is cancelled .
    +	 */
    +	public static final ConfigOption<Long> TASK_CANCELLATION_TIMEOUT_TIMERS = ConfigOptions
    +			.key("task.cancellation.timeout.timers")
    +			.defaultValue(7500L);
    --- End diff --
    
    add deprecated key? (I'm not quite sure whether the previous option was part of a release)


---

[GitHub] flink pull request #5402: [FLINK-8549] [config] Move TimerServiceOptions int...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/5402


---