Hi Agree with Ahmet, that even in that shape it should improve the queue length. Both _Commit/_Phrase cross-cancelling and "cancell all" phrase seem require much effort and I doubt it's worthy to do it.
I will turn on `Cancel build on update` in ghprb-plugin on ci-beam.apache.org tomorrow (Tuesday). Some discussions related to job filtering issue (or feature) in ghprb-plugin: https://github.com/jenkinsci/ghprb-plugin/issues/678 https://github.com/jenkinsci/ghprb-plugin/pull/680 BR Tobiasz On Fri, Jun 26, 2020 at 2:07 AM Ahmet Altay <al...@google.com> wrote: > > > On Thu, Jun 25, 2020 at 4:27 PM Tobiasz Kędzierski < > tobiasz.kedzier...@polidea.com> wrote: > >> Andrew thanks for great analysis +1 >> This bug with job filtering seems to be crucial to keep _Commit and >> _Phrase separate. >> >> I was considering the situation where the two PRs with the same commit >> hash are created. I created an artificial situation where two branches are >> identical and then two PRs with them. Two separate jobs were triggered. As >> you wrote, due to the matching GH status by job name and hash, both PR >> statuses were pointing to the same job (the newest one, which was wrong for >> one PR). As i tested, adding a new commit which will cancel the previous >> build would show false status on the PR with the previously wrong job link. >> It is possible to reproduce it, but could you give the real life >> situation where two jobs would be triggered with the same commit? >> I am asking because I think that enabling `Cancel build on update` may >> greatly improve Jenkins queue and it would be worthwhile to sacrifice this >> rare and unlikely case for it (if it is rare and unlikely case of course). >> > > I agree with this. > > >> >> Ahmet, I think the cancelling _Commit build by following _Phrase should >> be handled within ghprb-plugin if possible. I am not sure if we can make >> some workaround. Do you have any suggestions how we may solve it? >> > > I do not know jenkins enough to be able to make a good suggestion. We can > try: > - If it is possible to do this with ghprb plugin as you suggested, we can > do that. > - If not, we can make _Commit jobs cancel _Commit jobs only and _Phrase > jobs cancel _Phrase jobs only. It will still be an improvement. > > >> >> BR >> Tobiasz >> >> On Wed, Jun 24, 2020 at 12:28 AM Kenneth Knowles <k...@apache.org> wrote: >> >>> +1 to Andrew's analysis >>> >>> On Tue, Jun 23, 2020 at 12:13 PM Ahmet Altay <al...@google.com> wrote: >>> >>>> Would it be possible to cancel any running _Phrase or _Commit variants, >>>> if either one of them is triggered? >>>> >>>> On Tue, Jun 23, 2020 at 10:41 AM Andrew Pilloud <apill...@google.com> >>>> wrote: >>>> >>>>> I believe we split _Commit and _Phrase to work around a bug with job >>>>> filtering. For example, when you make a python change only the python >>>>> tests >>>>> are run based on the commit. We still want to be able to run the java jobs >>>>> by trigger phrase if needed. There are also performance tests (Nexmark for >>>>> example) that have different jobs to ensure PR runs don't end up published >>>>> in the performance dashboard, but i think those have a split of _Phrase >>>>> and >>>>> _Cron. >>>>> >>>>> As for canceling jobs, don't forget that the github status APIs are >>>>> keyed on commit hash and job name (not PR). It is possible for a commit to >>>>> be on multiple PRs and it is possible for a single PR to have >>>>> multiple commits. There are workflows that will be broken if you are >>>>> keying >>>>> off of a PR to automatically cancel jobs. >>>>> >>>>> On Tue, Jun 23, 2020 at 9:59 AM Tyson Hamilton <tyso...@google.com> >>>>> wrote: >>>>> >>>>>> +1 the ability to cancel in-flight jobs is worth deduplicating >>>>>> _Phrase and _Commit. I don't see a benefit for having both. >>>>>> >>>>>> On Tue, Jun 23, 2020 at 9:02 AM Luke Cwik <lc...@google.com> wrote: >>>>>> >>>>>>> I think this is a great improvement to prevent the Jenkins queue >>>>>>> from growing too large and has been suggested in the past but we were >>>>>>> unable to do due to difficulty with the version of the ghrpb plugin that >>>>>>> was used at the time. >>>>>>> >>>>>>> I know that we created different variants of the tests because we >>>>>>> wanted to track metrics based upon whether something was a post commit >>>>>>> (_Cron suffix) vs precommits but don't know why we split _Phrase and >>>>>>> _Commit. >>>>>>> >>>>>>> On Tue, Jun 23, 2020 at 3:35 AM Tobiasz Kędzierski < >>>>>>> tobiasz.kedzier...@polidea.com> wrote: >>>>>>> >>>>>>>> Hi everyone, >>>>>>>> >>>>>>>> I was investigating the possibility of canceling Jenkins builds >>>>>>>> when the update to PR makes prior build irrelevant. (related to >>>>>>>> https://issues.apache.org/jira/browse/BEAM-3105) >>>>>>>> In the `GitHub Pull Request Builder Jenkins plugin [ghprb-plugin] >>>>>>>> there is a hidden option `Cancel build on update` that seems to work >>>>>>>> fine. >>>>>>>> e.g. >>>>>>>> >>>>>>>> 1. >>>>>>>> >>>>>>>> I make a PR >>>>>>>> 2. >>>>>>>> >>>>>>>> ghprb-plugin triggers beam_PreCommit_PythonLint_Commit >>>>>>>> 3. >>>>>>>> >>>>>>>> I make a new commit to the PR >>>>>>>> >>>>>>>> 4. >>>>>>>> >>>>>>>> ghprb-plugin aborts the previous >>>>>>>> `beam_PreCommit_PythonLint_Commit` and adds to the queue the new >>>>>>>> one with >>>>>>>> updated sha1. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> This option seems to significantly improve the experience with >>>>>>>> build triggering and we are planning to enable it shortly. >>>>>>>> >>>>>>>> However, putting a phrase “Run PythonLint PreCommit” in the >>>>>>>> comment triggers new `beam_PreCommit_PythonLint_Phrase` build, but >>>>>>>> does not touch already queued or running >>>>>>>> `beam_PreCommit_PythonLint_Commit` >>>>>>>> builds, that are technically speaking, different jobs. >>>>>>>> >>>>>>>> For testing purposes I made a single job which was a “_Commit” job >>>>>>>> with added “Trigger phrase” and it works well (commit builds cancelled >>>>>>>> after putting phrase comment in PR) >>>>>>>> >>>>>>>> Hence my question: do we need separate “_Phrase” and “_Commit” jobs? >>>>>>>> >>>>>>>> BR >>>>>>>> Tobiasz >>>>>>>> >>>>>>> >> >> -- >> >> Tobiasz Kędzierski >> Polidea <https://www.polidea.com/> | Junior Software Engineer >> >> E: tobiasz.kedzier...@polidea.com >> [image: Polidea] <https://www.polidea.com/> >> >> Check out our projects! <https://www.polidea.com/our-work> >> [image: Github] <https://github.com/Polidea> [image: Facebook] >> <https://www.facebook.com/Polidea.Software> [image: Twitter] >> <https://twitter.com/polidea> [image: Linkedin] >> <https://www.linkedin.com/company/polidea> [image: Instagram] >> <https://instagram.com/polidea> [image: Behance] >> <https://www.behance.net/polidea> [image: dribbble] >> <https://dribbble.com/polideadesign> >> > -- Tobiasz Kędzierski Polidea <https://www.polidea.com/> | Junior Software Engineer E: tobiasz.kedzier...@polidea.com [image: Polidea] <https://www.polidea.com/> Check out our projects! <https://www.polidea.com/our-work> [image: Github] <https://github.com/Polidea> [image: Facebook] <https://www.facebook.com/Polidea.Software> [image: Twitter] <https://twitter.com/polidea> [image: Linkedin] <https://www.linkedin.com/company/polidea> [image: Instagram] <https://instagram.com/polidea> [image: Behance] <https://www.behance.net/polidea> [image: dribbble] <https://dribbble.com/polideadesign>
