[BEAM-6936] Added Jenkins jobs running Java examples on Dataflow with Java 11 #8183

mwalenia · 2019-04-01T14:07:58Z

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang	Apex	Dataflow	Flink	Gearpump	Samza	Spark
Go	---	---	---	---	---	---
Java
Python	---			---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

mwalenia · 2019-04-01T14:08:06Z

R: @lgajowy

lgajowy · 2019-04-02T09:34:05Z

Run seed job

lgajowy

Added suggestions. Thanks!

Other than that: I don't see the job results in Jenkins. Could you run them and check if they pass?

Could you also set the jira id to: BEAM-6936 in the commit message (it think there is a typo)

lgajowy · 2019-04-02T09:47:13Z

runners/google-cloud-dataflow-java/examples/build.gradle

  }
 }

+task postCommitFnApiWorkerJava11(type: Test) {


I see that this is a similar task to preCommitFnApiWorker. Could we reuse preCommitFnApiWorker and parametrize it? My main concern now is that we duplicate lots of code that will be hard to maintain.

When refactoring this, please remember to set default argument/parameter values to java 8's in order not to need to change anything else in the code that already uses this. Also please change its name to fnApiWorkerTests or something similar because it's clearly not preCommit only now.

After looking into this task implementation, I am unable to see any difference.

So I am also wondering, why we need that additional task at all. Wouldn't it be sufficient to just call the other?

So maybe rename preCommitFnApiWorkerto something like runFnApiWorker and call that task in respective Jenkins job definitions?

You're right, that didn't add anything. I renamed preCommitFnApiWorker to verifyFnApiWorker and removed my redundant tasks. Now both precommit (already existent) and postcommit (new) jobs use the same tasks. distinction between Java 11 and Java 8 is done by setting a flag in the Jenkins job

lgajowy · 2019-04-02T09:49:31Z

runners/google-cloud-dataflow-java/examples/build.gradle

+  }
+}
+
+task postCommitLegacyWorkerJava11(type: Test) {


Similarly to above, could we perform the same refactoring?

Unfortunately it wasn't possible. I couldn't find a way of extracting common configuration to another element. Setting system properties requires Test task type and those properties are restricted in scope to a single task - I experimented with setting a property in a task with dependsOn directive, but it wasn't effective. Do you know of another way of extracting this?
@adude3141 it's also a reply to your general remark, the solution you linked didn't work. I believe that currently the best way is to duplicate those tasks.

Maybe something like this will do?

diff --git a/runners/google-cloud-dataflow-java/examples/build.gradle b/runners/google-cloud-dataflow-java/examples/build.gradle index 72059bb064..f591e5009b 100644 --- a/runners/google-cloud-dataflow-java/examples/build.gradle +++ b/runners/google-cloud-dataflow-java/examples/build.gradle @@ -42,25 +42,33 @@ def dockerImageName = project(':beam-runners-google-cloud-dataflow-java').ext.do // If -PuseExecutableStage is set, the use_executable_stage_bundle_execution wil be enabled. def fnapiExperiments = project.hasProperty('useExecutableStage') ? 'beam_fn_api,use_executable_stage_bundle_execution' : "beam_fn_api" +// we require dataflowWorker, optional workerHarnessContainerImage and optional additionalOptions +def commonConfig = { dataflowWorkerJar, workerHarnessContainerImage = '', additionalOptions = [] -> + // return the preevaluated configuration closure + return { + testClassesDirs = files(project(":beam-examples-java").sourceSets.test.output.classesDirs) + include "**/WordCountIT.class" + include "**/WindowedWordCountIT.class" + forkEvery 1 + maxParallelForks 4 + + def preCommitBeamTestPipelineOptions = [ + "--project=${gcpProject}", + "--tempRoot=${gcsTempRoot}", + "--runner=TestDataflowRunner", + "--dataflowWorkerJar=${dataflowWorkerJar}", + "--workerHarnessContainerImage=${workerHarnessContainerImage}" + ] + additionalOptions + systemProperty "beamTestPipelineOptions", JsonOutput.toJson(preCommitBeamTestPipelineOptions) + } +} + task preCommitLegacyWorker(type: Test) { dependsOn ":beam-runners-google-cloud-dataflow-java-legacy-worker:shadowJar" def dataflowWorkerJar = project.findProperty('dataflowWorkerJar') ?: project(":beam-runners-google-cloud-dataflow-java-legacy-worker").shadowJar.archivePath - //Set workerHarnessContainerImage to empty to make Dataflow pick up the non-versioned container //image, which handles a staged worker jar. - def preCommitBeamTestPipelineOptions = [ - "--project=${gcpProject}", - "--tempRoot=${gcsTempRoot}", - "--runner=TestDataflowRunner", - "--dataflowWorkerJar=${dataflowWorkerJar}", - "--workerHarnessContainerImage=", - ] - testClassesDirs = files(project(":beam-examples-java").sourceSets.test.output.classesDirs) - include "**/WordCountIT.class" - include "**/WindowedWordCountIT.class" - forkEvery 1 - maxParallelForks 4 - systemProperty "beamTestPipelineOptions", JsonOutput.toJson(preCommitBeamTestPipelineOptions) + with commonConfig(dataflowWorkerJar) } task verifyFnApiWorker(type: Test) { @@ -68,20 +76,7 @@ task verifyFnApiWorker(type: Test) { dependsOn ":beam-runners-google-cloud-dataflow-java:buildAndPushDockerContainer" def dataflowWorkerJar = project.findProperty('dataflowWorkerJar') ?: project(":beam-runners-google-cloud-dataflow-java-fn-api-worker").shadowJar.archivePath - def preCommitBeamTestPipelineOptions = [ - "--project=${gcpProject}", - "--tempRoot=${gcsTempRoot}", - "--runner=TestDataflowRunner", - "--dataflowWorkerJar=${dataflowWorkerJar}", - "--workerHarnessContainerImage=${dockerImageName}", - "--experiments=${fnapiExperiments}", - ] - testClassesDirs = files(project(":beam-examples-java").sourceSets.test.output.classesDirs) - include "**/WordCountIT.class" - include "**/WindowedWordCountIT.class" - forkEvery 1 - maxParallelForks 4 - systemProperty "beamTestPipelineOptions", JsonOutput.toJson(preCommitBeamTestPipelineOptions) + with commonConfig(dataflowWorkerJar, dockerImageName, "--experiments=${fnapiExperiments}") useJUnit { excludeCategories 'org.apache.beam.sdk.testing.StreamingIT' } @@ -90,24 +85,10 @@ task verifyFnApiWorker(type: Test) { task postCommitLegacyWorkerJava11(type: Test) { dependsOn ":beam-runners-google-cloud-dataflow-java-legacy-worker:shadowJar" def dataflowWorkerJar = project.findProperty('dataflowWorkerJar') ?: project(":beam-runners-google-cloud-dataflow-java-legacy-worker").shadowJar.archivePath - //Set workerHarnessContainerImage to empty to make Dataflow pick up the non-versioned container //image, which handles a staged worker jar. - def preCommitBeamTestPipelineOptions = [ - "--project=${gcpProject}", - "--tempRoot=${gcsTempRoot}", - "--runner=TestDataflowRunner", - "--dataflowWorkerJar=${dataflowWorkerJar}", - "--workerHarnessContainerImage=", - ] - - testClassesDirs = files(project(":beam-examples-java").sourceSets.test.output.classesDirs) - include "**/WordCountIT.class" - include "**/WindowedWordCountIT.class" - forkEvery 1 - maxParallelForks 4 + with commonConfig(dataflowWorkerJar) systemProperty "java.specification.version", "11" - systemProperty "beamTestPipelineOptions", JsonOutput.toJson(preCommitBeamTestPipelineOptions) } task java11PostCommit() {

But please bear with me. It is a fast hack and I did not run. Also, if you decide to use, doublecheck that I did not accidentally deleted some required configuration.

Thanks for the suggestion, it worked with minimal modifications ;)

Nice. Glad I could help here!

lgajowy · 2019-04-02T09:57:46Z

Adding additional reviewer:

R: @adude3141

mwalenia · 2019-04-02T13:28:18Z

Run seed job

adude3141 · 2019-04-02T14:12:55Z

Also, as a general remark, I wonder whether it would not be easier to share common configurations. The different task configuration seems to be almost identical. Not sure, what would best be here, but maybe something like [1] will do?

Of course, sometimes it makes more sense to duplicate.

[1] https://stackoverflow.com/questions/13072034/avoid-duplication-between-similar-gradle-tasks

mwalenia · 2019-04-03T09:24:26Z

@lgajowy , @adude3141 I fixed what I could, WDYT?

mwalenia · 2019-04-03T09:48:15Z

Run seed job

adude3141 · 2019-04-04T10:09:44Z

runners/google-cloud-dataflow-java/examples/build.gradle

-          "--runner=TestDataflowRunner",
-          "--dataflowWorkerJar=${dataflowWorkerJar}",
-          "--workerHarnessContainerImage=${dockerImageName}",
-          "--experiments=${fnapiExperiments}",


Is this lost now?

Right, I missed it. Added it back

adude3141 · 2019-04-04T10:52:18Z

runners/google-cloud-dataflow-java/examples/build.gradle

+// we require dataflowWorker, optional workerHarnessContainerImage and optional additionalOptions
+def commonConfig = { dataflowWorkerJar, workerHarnessContainerImage = '', additionalOptions = [] ->
+   // return the preevaluated configuration closure
+           return {


will this ident pass spot less?

Good catch. It did but IMO it shouldn't have. It looks like no build.gradle files are checked only when applyGroovyNature() is invoked in them. Looks like a bug.

https://issues.apache.org/jira/browse/BEAM-7040 issue for this

@mwalenia could you fix it according to Intelij's suggestions?

adude3141 · 2019-04-04T10:54:06Z

From my point of view, it LGTM. Nice work. Thanks!

wdyt, @lgajowy ?

lgajowy

Some nits. Other than that please reorganize the branch (squash, etc) and we're good to go. Thanks!

lgajowy · 2019-04-09T09:30:24Z

runners/google-cloud-dataflow-java/examples/build.gradle

 // If -PuseExecutableStage is set, the use_executable_stage_bundle_execution wil be enabled.
 def fnapiExperiments = project.hasProperty('useExecutableStage') ? 'beam_fn_api,use_executable_stage_bundle_execution' : "beam_fn_api"

+// we require dataflowWorker, optional workerHarnessContainerImage and optional additionalOptions


Isn't this comment redundant? The line below shows exactly what the comment says but (and that's the risk) it can change in the future.

Please delete the comment

lgajowy · 2019-04-09T09:32:34Z

runners/google-cloud-dataflow-java/examples/build.gradle

 def fnapiExperiments = project.hasProperty('useExecutableStage') ? 'beam_fn_api,use_executable_stage_bundle_execution' : "beam_fn_api"

+// we require dataflowWorker, optional workerHarnessContainerImage and optional additionalOptions
+def commonConfig = { dataflowWorkerJar, workerHarnessContainerImage = '', additionalOptions = [] ->


Yep - this improves this code a lot. Great!

lgajowy · 2019-04-09T09:44:04Z

runners/google-cloud-dataflow-java/examples/build.gradle

+// we require dataflowWorker, optional workerHarnessContainerImage and optional additionalOptions
+def commonConfig = { dataflowWorkerJar, workerHarnessContainerImage = '', additionalOptions = [] ->
+   // return the preevaluated configuration closure
+           return {


Good catch. It did but IMO it shouldn't have. It looks like no build.gradle files are checked only when applyGroovyNature() is invoked in them. Looks like a bug.

lgajowy · 2019-04-09T09:48:27Z

runners/google-cloud-dataflow-java/examples/build.gradle

+// we require dataflowWorker, optional workerHarnessContainerImage and optional additionalOptions
+def commonConfig = { dataflowWorkerJar, workerHarnessContainerImage = '', additionalOptions = [] ->
+   // return the preevaluated configuration closure
+           return {


https://issues.apache.org/jira/browse/BEAM-7040 issue for this

@mwalenia could you fix it according to Intelij's suggestions?

lgajowy · 2019-04-09T09:57:33Z

runners/google-cloud-dataflow-java/examples/build.gradle

+  }
+}
+
+task postCommitLegacyWorkerJava11(type: Test) {


… Java 11

…ommit and postcommit

mwalenia · 2019-04-10T07:25:24Z

@lgajowy I fixed the nits and commit history. Ready to merge

lgajowy · 2019-04-10T09:35:26Z

Run seed job

lgajowy · 2019-04-10T09:47:46Z

Run Java examples on Dataflow with Java 11

lgajowy · 2019-04-10T09:48:27Z

Run Java Portability examples on Dataflow with Java 11

lgajowy

Tests are failing. Could you investigate/fix?

mwalenia · 2019-04-10T09:59:45Z

java.lang.RuntimeException: Workflow failed. Causes: Project apache-beam-testing has insufficient quota(s) to execute this workflow with 1 instances in region us-central1. Quota summary (required/available): 1/7244 instances, 4/0 CPUs, 430/116819 disk GB, 0/4046 SSD disk GB, 1/263 instance groups, 1/264 managed instance groups, 1/207 instance templates, 1/499 in-use IP addresses.
Something must have saturated CPU quota.

mwalenia · 2019-04-10T10:18:06Z

@alanmyrvold @pabloem There's a problem with tests exceeding CPU quota. I believe that we need to raise the quota, considering that we're adding more and more dataflow based tests. What do you think?

pabloem · 2019-04-11T23:51:50Z

Run Java Portability examples on Dataflow with Java 11

pabloem · 2019-04-11T23:52:42Z

quota issues should go away immediately. If we see them happening more often, we can investigate, but they should not be frequent.

pabloem · 2019-04-11T23:52:49Z

Run seed job

pabloem · 2019-04-12T16:14:25Z

Run seed job

pabloem · 2019-04-12T16:31:15Z

Run Java Portability examples on Dataflow with Java 11

pabloem · 2019-04-12T16:31:56Z

Run Java examples on Dataflow with Java 11

pabloem · 2019-04-12T16:54:05Z

: ) finally it's passing

pabloem · 2019-04-15T21:42:07Z

should we merge these? : P

lgajowy · 2019-04-16T09:45:47Z

Oops. Missed this. Sorry. Merging.

lgajowy · 2019-04-16T09:46:39Z

@mwalenia thanks for the PR.
@pabloem thanks for pinging me. :)

boyuanzz · 2020-01-17T23:16:17Z

runners/google-cloud-dataflow-java/examples/build.gradle

+               "--tempRoot=${gcsTempRoot}",
+               "--runner=TestDataflowRunner",
+               "--dataflowWorkerJar=${dataflowWorkerJar}",
+               workerHarnessContainerImage.isEmpty() ?'':"--workerHarnessContainerImage=${workerHarnessContainerImage}"


When dataflowWorkerJar is provided, the workerHarnessContainerImage should be set to empty explicitly, otherwise, the dataflow service will still try to pull harness image rather than using custom-built worker jar,

Fix here: #10635

lgajowy requested changes Apr 2, 2019

View reviewed changes

lgajowy changed the title ~~[BEAM-6963] Added Jenkins jobs running Java examples on Dataflow with Java 11~~ [BEAM-6936] Added Jenkins jobs running Java examples on Dataflow with Java 11 Apr 2, 2019

adude3141 reviewed Apr 4, 2019

View reviewed changes

lgajowy requested changes Apr 9, 2019

View reviewed changes

Michal Walenia added 2 commits April 10, 2019 09:19

[BEAM-6963] Added Jenkins jobs running Java examples on Dataflow with…

edc8138

… Java 11

[BEAM-6936] Refactor of the Portability API examples check, both prec…

210d172

…ommit and postcommit

mwalenia force-pushed the BEAM-6936-dataflow-java11-examples branch from fd81d7e to 210d172 Compare April 10, 2019 07:24

lgajowy requested changes Apr 10, 2019

View reviewed changes

lgajowy approved these changes Apr 16, 2019

View reviewed changes

lgajowy merged commit 4e099df into apache:master Apr 16, 2019

boyuanzz reviewed Jan 17, 2020

View reviewed changes

mwalenia deleted the BEAM-6936-dataflow-java11-examples branch January 24, 2020 10:40

[BEAM-6936] Added Jenkins jobs running Java examples on Dataflow with Java 11 #8183

[BEAM-6936] Added Jenkins jobs running Java examples on Dataflow with Java 11 #8183

Uh oh!

Conversation

mwalenia commented Apr 1, 2019

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

Uh oh!

mwalenia commented Apr 1, 2019

Uh oh!

lgajowy commented Apr 2, 2019

Uh oh!

lgajowy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgajowy commented Apr 2, 2019

Uh oh!

mwalenia commented Apr 2, 2019

Uh oh!

adude3141 commented Apr 2, 2019

Uh oh!

mwalenia commented Apr 3, 2019

Uh oh!

mwalenia commented Apr 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adude3141 Apr 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adude3141 commented Apr 4, 2019

Uh oh!

lgajowy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwalenia commented Apr 10, 2019

Uh oh!

lgajowy commented Apr 10, 2019

Uh oh!

lgajowy commented Apr 10, 2019

Uh oh!

lgajowy commented Apr 10, 2019

lgajowy left a comment •

edited

Loading

adude3141 Apr 4, 2019 •

edited

Loading