You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by kl0u <gi...@git.apache.org> on 2016/02/09 18:44:45 UTC

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

GitHub user kl0u opened a pull request:

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

    FLINK-2523: Makes the task cancellation interval configurable.

    

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

    $ git pull https://github.com/kl0u/flink task_cancellation_interval

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

    https://github.com/apache/flink/pull/1612.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 #1612
    
----
commit cd36d7ec883e69828bcb476d69aba465dca79b8d
Author: Aljoscha Krettek <al...@gmail.com>
Date:   2016-02-03T10:07:38Z

    [hotfix] Fix typos in Trigger.java

commit fdf74f036a96708fae7b8c8a5a2ce041bb7ed20f
Author: Kostas Kloudas <kk...@gmail.com>
Date:   2016-02-03T12:58:12Z

    FLINK-3327: Attaches the ExecutionConfig to the JobGraph and propagates it to the Task itself.

commit 5ead1c05215f4fd70f55370f7864674d670b1282
Author: Kostas Kloudas <kk...@gmail.com>
Date:   2016-02-09T14:48:00Z

    FLINK-2523: Makes the task cancellation interval configurable through the ExecutionConfig.

----


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:

    https://github.com/apache/flink/pull/1612#issuecomment-184236866
  
    Hello! Just rebased to the new master. Please review.


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52388350
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/deployment/TaskDeploymentDescriptor.java ---
    @@ -141,9 +146,16 @@ public TaskDeploymentDescriptor(
     			List<BlobKey> requiredJarFiles, List<URL> requiredClasspaths,
     			int targetSlotNumber) {
     
    -		this(appId, jobID, vertexID, executionId, taskName, indexInSubtaskGroup, numberOfSubtasks, attemptNumber,
    -				jobConfiguration, taskConfiguration, invokableClassName, producedPartitions,
    -				inputGates, requiredJarFiles, requiredClasspaths, targetSlotNumber, null, -1);
    +		this(appId, jobID, vertexID, executionId, new ExecutionConfig(), taskName, indexInSubtaskGroup,
    --- End diff --
    
    This constructor only exists for tests. The executionConfig in the TDD is the change of this pull request, so here the new ExecutionConfig is just to not break the already existing tests that were testing other parts of the functionality of the TDD. New tests are added to test the new functionality.


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52385934
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID, the given name, and the default
    +	 * {@link ExecutionConfig} parameters.
    +	 *
    +	 * @param jobID The id of the job. A random ID is generated, if {@code null} is passed.
    +	 * @param jobName The name of the job.
    +	 */
    +	public JobGraph(JobID jobID, String jobName) {
    +		this(jobID, jobName, new ExecutionConfig());
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name and a random job ID if null supplied as an id.
    +	 * Constructs a new job graph with the given name, the given {@link ExecutionConfig},
    +	 * and a random job ID.
    +	 *
    +	 * @param jobName The name of the job.
    +	 * @param config The execution configuration of the job.
    +	 */
    +	public JobGraph(String jobName, ExecutionConfig config) {
    +		this(null, jobName, config);
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID (or a random ID, if {@code null} is passed),
    +	 * the given name and the given execution configuration (see {@link ExecutionConfig}).
     	 *
     	 * @param jobId The id of the job. A random ID is generated, if {@code null} is passed.
     	 * @param jobName The name of the job.
    +	 * @param config The execution configuration of the job.
     	 */
    -	public JobGraph(JobID jobId, String jobName) {
    +	public JobGraph(JobID jobId, String jobName, ExecutionConfig config) {
     		this.jobID = jobId == null ? new JobID() : jobId;
     		this.jobName = jobName == null ? "(unnamed job)" : jobName;
    +		this.executionConfig = config;
    +	}
    +
    +	/**
    +	 * Constructs a new job graph containing the provided <b>single</b> {@code JobVertex} (see {@link JobVertex}),
    +	 * with no name, a random ID, and the default execution configuration (see {@link ExecutionConfig}).
    +	 *
    +	 * @param vertex The single vertex of the graph.
    +	 * */
    +	public JobGraph(JobVertex vertex) {
    +		this(null, Collections.singletonList(vertex));
    +	}
    +
    +	/**
    +	 * Constructs a new job graph containing the provided <b>single</b> {@code JobVertex} (see {@link JobVertex}),
    +	 * with the given name, a random ID, and the default execution configuration (see {@link ExecutionConfig}).
    +	 *
    +	 * @param jobName The name of the job.
    +	 * @param vertex The single vertex of the graph.
    +	 * */
    +	public JobGraph(String jobName, JobVertex vertex) {
    +		this(jobName, Collections.singletonList(vertex));
    +	}
    +
    +	/**
    +	 * Constructs a new job graph containing the provided <b>two</b> {@code JobVertices} (see {@link JobVertex}),
    +	 * with the given name, a random ID, and the default execution configuration (see {@link ExecutionConfig}).
    +	 *
    +	 * @param jobName The name of the job.
    +	 * @param vertex1 The first vertex of the graph.
    +	 * @param vertex2 The second vertex of the graph.
    +	 * */
    +	public JobGraph(String jobName, JobVertex vertex1, JobVertex vertex2) {
    +		this(jobName, Arrays.asList(vertex1, vertex2));
     	}
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID if null supplied as an id.
    +	 * Constructs a new job graph containing the provided {@code JobVertices} (see {@link JobVertex}),
    +	 * with no name, a random job ID, and the default execution configuration (see {@link ExecutionConfig}).
     	 *
     	 * @param vertices The vertices to add to the graph.
     	 */
    -	public JobGraph(JobVertex... vertices) {
    --- End diff --
    
    Why did you remove the varargs constructor?


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

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


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52385415
  
    --- Diff: docs/apis/batch/index.md ---
    @@ -2123,7 +2123,7 @@ Note that types registered with `registerKryoType()` are not available to Flink'
     
     - `disableAutoTypeRegistration()` Automatic type registration is enabled by default. The automatic type registration is registering all types (including sub-types) used by usercode with Kryo and the POJO serializer.
     
    -
    +- `setTaskCancellationInterval(long interval)` Sets the the interval (in milliseconds) to wait between consecutive attempts to cancel a running task. By default this is set to **30000** milliseconds, or **30 seconds**.
    --- End diff --
    
    I would explain a bit more detailed what's going on. The `cancel()` method is called only once. The interval determines the interrupt frequency! 


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52356401
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID, the given name, and the default
    +	 * {@link ExecutionConfig} parameters.
    +	 *
    +	 * @param jobID The id of the job. A random ID is generated, if {@code null} is passed.
    +	 * @param jobName The name of the job.
    +	 */
    +	public JobGraph(JobID jobID, String jobName) {
    +		this(jobID, jobName, new ExecutionConfig());
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name and a random job ID if null supplied as an id.
    +	 * Constructs a new job graph with the given name, the given {@link ExecutionConfig},
    +	 * and a random job ID.
    +	 *
    +	 * @param jobName The name of the job.
    +	 * @param config The execution configuration of the job.
    +	 */
    +	public JobGraph(String jobName, ExecutionConfig config) {
    +		this(null, jobName, config);
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID (or a random ID, if {@code null} is passed),
    +	 * the given name and the given execution configuration (see {@link ExecutionConfig}).
     	 *
     	 * @param jobId The id of the job. A random ID is generated, if {@code null} is passed.
     	 * @param jobName The name of the job.
    +	 * @param config The execution configuration of the job.
     	 */
    -	public JobGraph(JobID jobId, String jobName) {
    +	public JobGraph(JobID jobId, String jobName, ExecutionConfig config) {
     		this.jobID = jobId == null ? new JobID() : jobId;
     		this.jobName = jobName == null ? "(unnamed job)" : jobName;
    +		this.executionConfig = config;
    --- End diff --
    
    Can `executionConfig` be `null`? If not, then we should insert a check here.


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52385223
  
    --- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGeneratorTest.java ---
    @@ -27,13 +24,17 @@
     import org.apache.flink.streaming.api.datastream.DataStream;
     import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment;
     import org.apache.flink.streaming.api.functions.sink.SinkFunction;
    -import org.apache.flink.util.InstantiationUtil;
     
    +import org.apache.flink.util.InstantiationUtil;
     import org.junit.Test;
     
    +import java.io.IOException;
    +import java.util.Random;
    +
     import static org.junit.Assert.*;
     
    -public class StreamingJobGraphGeneratorTest {
    +public class
    +StreamingJobGraphGeneratorTest {
    --- End diff --
    
    why is there a line break after the `class`?


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52388927
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID, the given name, and the default
    +	 * {@link ExecutionConfig} parameters.
    +	 *
    +	 * @param jobID The id of the job. A random ID is generated, if {@code null} is passed.
    +	 * @param jobName The name of the job.
    +	 */
    +	public JobGraph(JobID jobID, String jobName) {
    +		this(jobID, jobName, new ExecutionConfig());
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name and a random job ID if null supplied as an id.
    +	 * Constructs a new job graph with the given name, the given {@link ExecutionConfig},
    +	 * and a random job ID.
    +	 *
    +	 * @param jobName The name of the job.
    +	 * @param config The execution configuration of the job.
    +	 */
    +	public JobGraph(String jobName, ExecutionConfig config) {
    +		this(null, jobName, config);
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID (or a random ID, if {@code null} is passed),
    +	 * the given name and the given execution configuration (see {@link ExecutionConfig}).
     	 *
     	 * @param jobId The id of the job. A random ID is generated, if {@code null} is passed.
     	 * @param jobName The name of the job.
    +	 * @param config The execution configuration of the job.
     	 */
    -	public JobGraph(JobID jobId, String jobName) {
    +	public JobGraph(JobID jobId, String jobName, ExecutionConfig config) {
     		this.jobID = jobId == null ? new JobID() : jobId;
     		this.jobName = jobName == null ? "(unnamed job)" : jobName;
    +		this.executionConfig = config;
    --- End diff --
    
    It cannot, I just added a check.


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:

    https://github.com/apache/flink/pull/1612#issuecomment-182413649
  
    Thanks a lot for the comments @rmetzger and @tillrohrmann . I integrated them.
    Please review the new pull request. 
    This pull request is based upon the one about FLINK-3327.
    
    Thanks a lot!



---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52388853
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID, the given name, and the default
    +	 * {@link ExecutionConfig} parameters.
    +	 *
    +	 * @param jobID The id of the job. A random ID is generated, if {@code null} is passed.
    +	 * @param jobName The name of the job.
    +	 */
    +	public JobGraph(JobID jobID, String jobName) {
    +		this(jobID, jobName, new ExecutionConfig());
    --- End diff --
    
    Same as before.


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52386582
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    +	}
    +
    +	/**
    +	 * Constructs a new job graph with the given job ID, the given name, and the default
    +	 * {@link ExecutionConfig} parameters.
    +	 *
    +	 * @param jobID The id of the job. A random ID is generated, if {@code null} is passed.
    +	 * @param jobName The name of the job.
    +	 */
    +	public JobGraph(JobID jobID, String jobName) {
    +		this(jobID, jobName, new ExecutionConfig());
    --- End diff --
    
    same Q here


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52385188
  
    --- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGeneratorTest.java ---
    @@ -75,16 +76,22 @@ public void testExecutionConfigSerialization() throws IOException, ClassNotFound
     			config.disableSysoutLogging();
     		}
     		config.setParallelism(dop);
    -		
    +
     		JobGraph jobGraph = compiler.createJobGraph("test");
    -		
    +
    +		final String exec_config_key = "runtime.execconfig";
    --- End diff --
    
    variable name is not java style


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52386295
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    --- End diff --
    
    Why are you creating an empty execution config here?
    
    Is this the execution config which will be passed to the user in the end?


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52388830
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
    @@ -102,60 +101,150 @@
     	// --------------------------------------------------------------------------------------------
     
     	/**
    -	 * Constructs a new job graph with no name and a random job ID.
    +	 * Constructs a new job graph with no name, a random job ID, and the default
    +	 * {@link ExecutionConfig} parameters.
     	 */
     	public JobGraph() {
     		this((String) null);
     	}
     
     	/**
    -	 * Constructs a new job graph with the given name, a random job ID.
    +	 * Constructs a new job graph with the given name, a random job ID and the default
    +	 * {@link ExecutionConfig} parameters.
     	 *
     	 * @param jobName The name of the job
     	 */
     	public JobGraph(String jobName) {
    -		this(null, jobName);
    +		this(null, jobName, new ExecutionConfig());
    --- End diff --
    
    Again here the constructors that do not specify executionConfigs are only exists for tests. So here the empty ExecutionConfig is just to not break the already existing tests that were testing other parts of the functionality.


---
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.
---

[GitHub] flink pull request: FLINK-2523: Makes the task cancellation interv...

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

    https://github.com/apache/flink/pull/1612#discussion_r52385698
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/deployment/TaskDeploymentDescriptor.java ---
    @@ -141,9 +146,16 @@ public TaskDeploymentDescriptor(
     			List<BlobKey> requiredJarFiles, List<URL> requiredClasspaths,
     			int targetSlotNumber) {
     
    -		this(appId, jobID, vertexID, executionId, taskName, indexInSubtaskGroup, numberOfSubtasks, attemptNumber,
    -				jobConfiguration, taskConfiguration, invokableClassName, producedPartitions,
    -				inputGates, requiredJarFiles, requiredClasspaths, targetSlotNumber, null, -1);
    +		this(appId, jobID, vertexID, executionId, new ExecutionConfig(), taskName, indexInSubtaskGroup,
    --- End diff --
    
    why is there a constructor variant of the TDD that creates an empty execution config?


---
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.
---