[BEAM-6248] Add Flink v1.7 build target to Flink Runner #7300
Conversation
|
Running three Flink targets in parallel seems to cause problems with JVM exists. We might have to let them depend on each other to avoid test execution problems. Will investigate. |
|
Run Java PreCommit |
|
Run Portable_Python PreCommit |
|
Testing issues resolved via #7309. Failures are now due to https://builds.apache.org/job/beam_PreCommit_Java_Commit/3183/console |
|
There is #7317 to address the Hadoop tests. Since test failures are unrelated, this is ready to be reviewed. |
|
Run Python PreCommit |
|
Run Java PreCommit |
|
Out of memory exception. We might have to run the tests across the different versions sequentially. |
|
Run Java PreCommit |
|
Would like to merge #7333 before merging this. The problem fixed in that PR became more visible with the Flink versions running in parallel. |
runners/flink/1.7/build.gradle
Outdated
| /* All properties required for loading the Flink build script */ | ||
| project.ext { | ||
| // Set the version of all Flink-related dependencies here. | ||
| flink_version = '1.7.0' |
|
You mentioned that tests for multiple Flink versions should run sequentially. Are you going to address it as part of this PR? |
Yes, this has to be done in this PR. Otherwise, we get OutOfMemoryException due to three Flink clusters running. I haven't come up with an elegant way of doing it. Alternatively, decreasing the parallelism could also help, it's usually 16 on Jenkins. |
|
Run Java PreCommit |
mxm
force-pushed
the
BEAM-6248
branch
2 times, most recently
from
7d00dd0
to
3a8c874
Compare
|
Limiting the parallelism seems to have cured the Jenkins test problems. |
|
Run Python PreCommit |
| @@ -86,6 +86,7 @@ public void testExecution() throws Exception { | |||
| options.setRunner(CrashingRunner.class); | |||
| options.as(FlinkPipelineOptions.class).setFlinkMaster("[local]"); | |||
| options.as(FlinkPipelineOptions.class).setStreaming(isStreaming); | |||
| options.as(FlinkPipelineOptions.class).setParallelism(4); | |||
There was a problem hiding this comment.
Good find. We should always set the parallelism to cap resource consumption. I would prefer 2 if the intention is to exercise parallel execution path, 1 otherwise.
There was a problem hiding this comment.
Our current test setup wastes quite a few resources because it uses the number of available cores in most places. Will do a follow-up to fix that.
I would prefer 2 if the intention is to exercise parallel execution path, 1 otherwise.
Do you think a slightly higher parallelism would increase the chance of exposing bugs? For example, the key distribution might be more skewed with a higher parallelism.
There was a problem hiding this comment.
To give a meaningful signal, the test should probably be different/specialized and assert something about the parallel execution? If not, then we can probably just set it to 1 ? As it stands we are operating on hope :)
There was a problem hiding this comment.
Testing to a large degree is based on hope to expose bugs while testing functionality :) We have seen tests running fine with parallelism 1 in the past while they fail with parallelism >1. ValidatesRunner for instance still runs with parallelism 1. But I'm ok with lowering the parallelism to 2.
|
I think this could be included in the 2.10 release, as it would help more users to try out Beam. |
|
Run Java PreCommit |
1 similar comment
|
Run Java PreCommit |
|
@mxm please backport and resolve JIRA: https://issues.apache.org/jira/browse/BEAM-6248 |
|
Backported to |
This adds a build target for Flink 1.7.1.
Post-Commit Tests Status (on master branch)