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). 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? 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>
