On Thu, Jun 25, 2020 at 4:27 PM Tobiasz Kędzierski < [email protected]> 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 <[email protected]> wrote: > >> +1 to Andrew's analysis >> >> On Tue, Jun 23, 2020 at 12:13 PM Ahmet Altay <[email protected]> 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 <[email protected]> >>> 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 <[email protected]> >>>> 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 <[email protected]> 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 < >>>>>> [email protected]> 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: [email protected] > [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> >
