Skip to content

Conversation

@Abacn
Copy link
Contributor

@Abacn Abacn commented Feb 23, 2023

It is found when reviewing #24015, whenever a new commit pushed, all (65) precommits are triggered at once and it simply causes Jenkins send too much GitHub API, in the following minutes:

  • other PR commit push does not trigger jenkins job run in time
  • finished jenkins job run stuck at reporting status to GitHub
  • phrase not working

Many of these "PreCommit" are splitted from old big JavaPreCommit (for unit test) and JavaPostCommit (for integration tests) #24584. Currently they are triggered automatically when the change is made on java core or buildSrc. I had the concern if this could cause problem when reviewing that PR, apparently it is true. Given that our build system cannot handle these number of PreCommit run at the same time, here I propose disable triggering most of these IO_Direct (21) precommit on buildSrc and core change.

The Con is that when there is a change on dependency version (handled by BeamModulePlugin) one needs to trigger the job by commenting on PR manually now. Considering that every precommit is running every 6 hr and there is another gate on release process (#25609) this should be fine.

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)

Build python source distribution and wheels
Python tests
Java tests
Go tests

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

@github-actions github-actions bot added the infra label Feb 23, 2023
@Abacn
Copy link
Contributor Author

Abacn commented Feb 23, 2023

@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@Abacn
Copy link
Contributor Author

Abacn commented Feb 23, 2023

run seed job

@Abacn
Copy link
Contributor Author

Abacn commented Feb 23, 2023

(seed job not triggering, currently jenkins is experience the issues described above, after another push of #24015 done 1 hr ago

Update: seed job succeeded: https://ci-beam.apache.org/job/beam_SeedJob/11161/

@damccorm
Copy link
Contributor

I wonder if a better fix here is to consolidate some of our sharded precommits so that instead of having 1 precommit for IOs (our old state) or 21 precommits for IOs (our current state) we have a precommit per IO if the IO changes frequently and we have 1 precommit (or a few precommits) for the remaining relatively stable ones.

A completely different alternative would be to have individual IO suites that trigger when that IOs directory is modified and 1 "mega IO" suite that triggers when we want to run all of our IO tests. That has the downside of getting out of sync more easily, but does what we want about as efficiently and precisely as possible.

All of this is more of a test structure question for a set of suites I don't personally care a ton about, so I'll defer to you, other IO folks, and @kennknowles (who did the initial sharding, which has been a win IMO), but I don't love the idea of not running precommits that have the potential to break on dependency changes. I also acknowledge, though, that all of these solutions stink because the real solution is to actually fix the infrastructure (which probably means move to GHA for us), but that's not a realistic stopgap solution.

@github-actions github-actions bot added the build label Feb 23, 2023
@Abacn Abacn force-pushed the constriantrigger branch 2 times, most recently from 15e0461 to 09988b7 Compare February 23, 2023 23:04
@Abacn
Copy link
Contributor Author

Abacn commented Feb 23, 2023

run seed job

@Abacn
Copy link
Contributor Author

Abacn commented Feb 23, 2023

run seed job

Update: passed: https://ci-beam.apache.org/job/beam_SeedJob/11169/

@Abacn
Copy link
Contributor Author

Abacn commented Feb 23, 2023

Thanks @damccorm. Created a "mega" suite only for code change on buildSrc/core/io common; in addition to 21 io precommits on particular IOs

Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach

@Abacn
Copy link
Contributor Author

Abacn commented Feb 27, 2023

I'm going to merging this as no further change request received. Summary:

For Cron jobs or PR changes only involving specific IO, nothing changed

For PR changes involving

  • Default triggers (buildSrc, gradle, settings)
  • Java core
  • Java IO common

Before: trigger all IO precommits

After: build task of 21 IO precommits combined into a single precommit task

@Abacn Abacn merged commit 7db76df into apache:master Feb 27, 2023
@Abacn Abacn deleted the constriantrigger branch February 27, 2023 23:00
ruslan-ikhsan pushed a commit to akvelon/beam that referenced this pull request Mar 10, 2023
…ge (apache#25612)

* Disable triggering most IO_Direct precommit on buildSrc and core change

* Add jobs to README

* Create an umbrella IO job

* Fix readme: IOs job does not have cron
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants