Add Gradle sub-projects to enable parallel running of Kafka tests #26153

johnjcasey · 2023-04-06T16:41:57Z

Please add a meaningful description for your change here

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

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

johnjcasey · 2023-04-06T16:43:33Z

Run Java_Kafka_IO_Direct PreCommit

…ka tests

johnjcasey · 2023-04-10T15:05:33Z

Run Java_Kafka_IO_Direct PreCommit

johnjcasey · 2023-04-10T16:56:13Z

Run Java_Kafka_IO_Direct PreCommit

…projects to enable running tests in parallel

codecov · 2023-04-11T18:37:56Z

Codecov Report

Merging #26153 (b2035f2) into master (a82a49b) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head b2035f2 differs from pull request most recent head 63633ca. Consider uploading reports for the commit 63633ca to get more accurate results

@@            Coverage Diff             @@
##           master   #26153      +/-   ##
==========================================
- Coverage   71.17%   71.15%   -0.02%     
==========================================
  Files         787      787              
  Lines      103293   103316      +23     
==========================================
+ Hits        73516    73518       +2     
- Misses      28288    28309      +21     
  Partials     1489     1489

Flag	Coverage Δ
python	`79.84% <ø> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2023-04-12T16:37:25Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

johnjcasey · 2023-04-12T18:00:47Z

run python_runners precommit

johnjcasey · 2023-04-12T18:01:00Z

run python_coverage precommit

johnjcasey · 2023-04-12T18:01:09Z

run java precommit

johnjcasey · 2023-04-12T18:01:26Z

run java_gcp_io_direct precommit

johnjcasey · 2023-04-12T18:02:33Z

java precommit failure is a flake

Abacn · 2023-04-12T18:23:37Z

sdks/java/io/kafka/kafka-100/build.gradle

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import org.apache.beam.gradle.kafka.KafkaTestUtilities


We can have a common.gradle in sdks/java/io/kafka and then build.gradle in kafka-{version} refers to it, like what flink runner support doing: https://github.com/apache/beam/blob/master/runners/flink/1.12/build.gradle

github-actions · 2023-04-12T18:35:30Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @Abacn for label java.
R: @damccorm for label build.
R: @ahmedabu98 for label io.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

johnjcasey · 2023-04-12T18:52:50Z

GCP IO test failure is unrelated. We might have a quota issue on Spanner tests?

jrmccluskey · 2023-04-12T19:04:45Z

That's a known issue with spanner permissions #26208

johnjcasey · 2023-04-12T22:24:06Z

All 4 failures appear to be unrelated flakes

damondouglas · 2023-04-12T22:33:12Z

buildSrc/src/main/groovy/org/apache/beam/gradle/kafka/KafkaTestUtilities.groovy

+        '"numRecords": "1000",' +
+        '"keySizeBytes": "10",' +
+        '"valueSizeBytes": "90"' +
+        '}',
+        "--readTimeout=120",
+        "--kafkaTopic=beam",
+        "--withTestcontainers=true",
+        "--kafkaContainerVersion=5.5.2",


Could we have these as variables that are then concatenated in the pipelineOptions?

damccorm · 2023-04-13T13:05:52Z

sdks/java/io/kafka/build.gradle

+//Because this runs many integration jobs in parallel, each of which use a
+//container, it can fail locally due to performance limitations on a desktop.
+//To avoid this, use --max-workers=N, where N is less than half your CPUs.
+//4 is a good start for parallelism without overloading your computer.


This is concerning to me, especially because Jenkins runs several jobs at the same time. If you run 2 of these jobs (or even worse, 4) on the same Jenkins machine, does the worker fall over?

I wonder if there's a version of this change that does some parallelization by default, but not this much (e.g. it spins up at most 4 containers at once)

They are running fine on the Jenkins workers right now. I've tried to limit parallelism to just one sub-project, but I can only manage to configure the entire build to have a max number of workes

I'll take another stab at this

yeah, it looks like I can't configure parallelism differently for a subproject.

I could add artificial dependencies between some of these tasks, to try and limit parallelism, but that would be a janky fix

Ok - I can live without it at first. If we see these tests failing (at a higher than normal rate) with Jenkins infra-like issues, we should consider clawing this back though

damccorm · 2023-04-13T13:09:10Z

sdks/java/io/kafka/kafka-01103/build.gradle

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import org.apache.beam.gradle.kafka.KafkaTestUtilities


Rather than having all of these in individual files, could we do this all in a loop from the base gradle file with something like

beam/runners/spark/3/build.gradle

Line 53 in 2325634

tasks.register("sparkVersion${kv.key}Test", Test) {

Nope. That is essentially what we had before, and we don't get parallelism with that because tasks within a project can't be run in parallel

I like the refactor here

Mostly, I wanted to avoid the same lines being repeated and you accomplished that with the shared properties file

damccorm

LGTM, but we should definitely monitor these for causing infra failures

johnjcasey · 2023-04-13T16:49:35Z

all failures are unrelated flakes

…ache#26153) * add parallel number to gradle.properties * remove unused config, attempt to configure higher parallelism for kafka tests * Create generic task to represent Integration tests. Then, create sub-projects to enable running tests in parallel * extend test timeout to reduce flakyness * add comment for running locally * run spotless * factor kafka integration tests up a level (cherry picked from commit 837733e)

…6153) (#26263) * add parallel number to gradle.properties * remove unused config, attempt to configure higher parallelism for kafka tests * Create generic task to represent Integration tests. Then, create sub-projects to enable running tests in parallel * extend test timeout to reduce flakyness * add comment for running locally * run spotless * factor kafka integration tests up a level (cherry picked from commit 837733e)

add parallel number to gradle.properties

c6dc54e

remove unused config, attempt to configure higher parallelism for kaf…

8c8dc08

…ka tests

github-actions bot added io java kafka labels Apr 10, 2023

Create generic task to represent Integration tests. Then, create sub-…

b2035f2

…projects to enable running tests in parallel

github-actions bot added the build label Apr 11, 2023

johnjcasey added 2 commits April 12, 2023 09:55

extend test timeout to reduce flakyness

76ec01c

add comment for running locally

a4ba8be

johnjcasey marked this pull request as ready for review April 12, 2023 15:46

run spotless

c92325f

Abacn reviewed Apr 12, 2023

View reviewed changes

github-actions bot added the Next Action: Reviewers label Apr 12, 2023

johnjcasey changed the title ~~add parallel number to gradle.properties~~ Add Gradle sub-projects to enable parallel running of Apr 12, 2023

johnjcasey changed the title ~~Add Gradle sub-projects to enable parallel running of~~ Add Gradle sub-projects to enable parallel running of Kafka tests Apr 12, 2023

damondouglas requested changes Apr 12, 2023

View reviewed changes

damccorm reviewed Apr 13, 2023

View reviewed changes

factor kafka integration tests up a level

63633ca

damccorm approved these changes Apr 13, 2023

View reviewed changes

johnjcasey merged commit 837733e into apache:master Apr 13, 2023

Abacn mentioned this pull request Apr 18, 2023

[Failing Test]: beam_PreCommit_Java_Kafka_IO_Direct flaky: KafkaIOIT.testWatermarkUpdateWithSparseMessages #25261

Closed

15 tasks

Abacn mentioned this pull request May 5, 2023

Add Gradle sub-projects to enable parallel running of Kafka tests (#2… #26263

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Gradle sub-projects to enable parallel running of Kafka tests #26153

Add Gradle sub-projects to enable parallel running of Kafka tests #26153

johnjcasey commented Apr 6, 2023

johnjcasey commented Apr 6, 2023

johnjcasey commented Apr 10, 2023

johnjcasey commented Apr 10, 2023

codecov bot commented Apr 11, 2023 •

edited

Loading

github-actions bot commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

Abacn Apr 12, 2023

github-actions bot commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

jrmccluskey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

damondouglas Apr 12, 2023

damccorm Apr 13, 2023

johnjcasey Apr 13, 2023

johnjcasey Apr 13, 2023

johnjcasey Apr 13, 2023

johnjcasey Apr 13, 2023

damccorm Apr 13, 2023

damccorm Apr 13, 2023

johnjcasey Apr 13, 2023

damccorm Apr 13, 2023

damccorm Apr 13, 2023

damccorm left a comment

johnjcasey commented Apr 13, 2023

Add Gradle sub-projects to enable parallel running of Kafka tests #26153

Add Gradle sub-projects to enable parallel running of Kafka tests #26153

Conversation

johnjcasey commented Apr 6, 2023

GitHub Actions Tests Status (on master branch)

johnjcasey commented Apr 6, 2023

johnjcasey commented Apr 10, 2023

johnjcasey commented Apr 10, 2023

codecov bot commented Apr 11, 2023 • edited Loading

Codecov Report

github-actions bot commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

Choose a reason for hiding this comment

github-actions bot commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

jrmccluskey commented Apr 12, 2023

johnjcasey commented Apr 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damccorm left a comment

Choose a reason for hiding this comment

johnjcasey commented Apr 13, 2023

codecov bot commented Apr 11, 2023 •

edited

Loading