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/03 19:39:02 UTC
[GitHub] flink pull request: ExecutionConfig to JobGraph.
GitHub user kl0u opened a pull request:
https://github.com/apache/flink/pull/1583
ExecutionConfig to JobGraph.
This makes the ExecutionConfig available to the Task. In a nutshell, the ExecutionConfig is attached to the JobGraph which is sent to the JobManager. The JobManager passes it to the ExecutionGraph, and, later on, to the TaskDeploymentDescriptor, and the TaskManager puts it into the Environment, which is visible to the AbstractInvocable.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kl0u/flink config_to_jobgraph
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1583.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 #1583
----
commit 58308ddd7be38d9019c77738e4faebf05c92cc12
Author: Aljoscha Krettek <al...@gmail.com>
Date: 2016-02-03T10:07:38Z
[hotfix] Fix typos in Trigger.java
commit ce3645d3d965a063696af3c56ca9cb0b8be3b36c
Author: Kostas Kloudas <kk...@gmail.com>
Date: 2016-02-03T12:58:12Z
Attaches the ExecutionConfig to the JobGraph and propagates it to the Task itself.
----
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-195540338
merging this.
---
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: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-181917384
Thanks a lot @StephanEwen
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-185210139
Hello! I just rebased to the new master.
Please review this new PR.
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-184259383
Made a few remarks, but didn't find any issue that isn't a minor one. Good job!
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-193330011
@StephanEwen , @zentol I just rebased to the latest master.
It passes all tests on my Travis (https://travis-ci.org/kl0u/flink/builds/114228524).
Please review because the PR touches a lot of classes.
---
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-3327: ExecutionConfig to JobGraph.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1583
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52912014
--- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala ---
@@ -1051,7 +1052,8 @@ class JobManager(
}
catch {
case t: Throwable =>
- log.error(s"Failed to submit job $jobId ($jobName)", t)
+ val message = t.getMessage
+ log.error(s"Failed to submit job $jobId ($jobName): $message", t)
--- End diff --
does the exception's message not show up in the logs without this change?
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52912601
--- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGeneratorTest.java ---
@@ -75,16 +75,22 @@ public void testExecutionConfigSerialization() throws IOException, ClassNotFound
config.disableSysoutLogging();
}
config.setParallelism(dop);
-
+
JobGraph jobGraph = compiler.createJobGraph("test");
-
+
+ final String exec_config_key = "runtime.config";
+
+ InstantiationUtil.writeObjectToConfig(jobGraph.getExecutionConfig(),
+ jobGraph.getJobConfiguration(),
+ exec_config_key);
+
ExecutionConfig executionConfig = InstantiationUtil.readObjectFromConfig(
jobGraph.getJobConfiguration(),
- ExecutionConfig.CONFIG_KEY,
+ exec_config_key,
Thread.currentThread().getContextClassLoader());
-
+
assertNotNull(executionConfig);
-
--- End diff --
this class contains a relatively high number of pure formatting changes.
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-195605736
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-182414285
Please review the new pull request.
This pull request is the first step for the one about FLINK-2523.
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52911077
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
@@ -215,7 +218,12 @@ public void setNumberOfExecutionRetries(int numberOfExecutionRetries) {
* @return The number of times the system will try to re-execute failed tasks.
*/
public int getNumberOfExecutionRetries() {
- return numExecutionRetries;
+ int retries = executionConfig.getNumberOfExecutionRetries();
+ if (retries < -1) {
--- End diff --
this check shouldn't be necessary, since setNumberOfExecutionRetries already checks for it.
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-184252247
we could probably heavily reduce the diff by providing constructors with/without the executionconfig argument. (if none is given, we just instantiate a new one)
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-193337830
looks like something went wrong while rebasing, a commit by aljoscha suddenly popped up.
but hey, that can be fixed while merging, +1 from me.
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52910257
--- Diff: flink-clients/src/main/java/org/apache/flink/client/FlinkYarnSessionCli.java ---
@@ -393,7 +393,7 @@ public int run(String[] args) {
printUsage();
return 1;
}
-
+
--- End diff --
only change made in this file is formatting.
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-185830388
Please Review this PR.
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52912585
--- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGeneratorTest.java ---
@@ -75,16 +75,22 @@ public void testExecutionConfigSerialization() throws IOException, ClassNotFound
config.disableSysoutLogging();
}
config.setParallelism(dop);
-
+
JobGraph jobGraph = compiler.createJobGraph("test");
-
+
+ final String exec_config_key = "runtime.config";
--- End diff --
variable doesn't follow naming conventions
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-184236773
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-193338674
Thanks a lot @zentol
---
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: ExecutionConfig to JobGraph.
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-181908838
Looks good at a first glance. I'll try to go through it and see if I can merge it...
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-195034317
are there any objections to merging this? let's get this one in.
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52911198
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/JobGraph.java ---
@@ -225,22 +233,12 @@ public int getNumberOfExecutionRetries() {
* @return The delay of time in milliseconds the system will try to re-execute failed tasks.
*/
public long getExecutionRetryDelay() {
- return executionRetryDelay;
- }
-
- /**
- * Sets the delay that failed tasks are re-executed. A value of zero
- * effectively disables fault tolerance. A value of {@code -1} indicates that the system
- * default value (as defined in the configuration) should be used.
- *
- * @param executionRetryDelay The delay of time the system will wait to re-execute failed tasks.
- */
- public void setExecutionRetryDelay(long executionRetryDelay){
- if (executionRetryDelay < -1) {
+ long retryDelay = executionConfig.getExecutionRetryDelay();
+ if (retryDelay < -1) {
--- End diff --
same as above
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-185637076
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-185641724
Changes look good. Do you know what happened to the ExecutionStateProgressTest diff?
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-185734236
The problem is the CRLF vs LF characters in the committed versions of the files.
---
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-3327: ExecutionConfig to JobGraph.
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/1583#discussion_r52912765
--- Diff: flink-yarn-tests/src/main/java/org/apache/flink/yarn/YarnTestBase.java ---
@@ -112,7 +112,7 @@
* lib/ folder of the flink distribution.
*/
protected static File flinkLibFolder;
-
+
--- End diff --
only change in this file is formatting
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-187143786
Just rebased. Please review this PR.
---
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-3327: ExecutionConfig to JobGraph.
Posted by kl0u <gi...@git.apache.org>.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1583#issuecomment-185271802
Hello!
I just rebased to the new master.
Please review this new PR.
---
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.
---