Yes, thanks. Really helpful. Kenn
On Tue, Jul 14, 2020 at 9:48 AM Ahmet Altay <al...@google.com> wrote: > Great news! Thank you for the update and making this happen! > > On Tue, Jul 14, 2020 at 1:09 AM Tobiasz Kędzierski < > tobiasz.kedzier...@polidea.com> wrote: > >> I think the experiment went quite well. >> Now all nodes have the increased number of the executors [1].. >> Canceling Jenkins builds after PR updates seems to work as well. Queue is >> usually very short. >> >> [1]: https://ci-beam.apache.org/label/beam/load-statistics?type=hour >> >> BR >> Tobiasz >> >> On Mon, Jul 6, 2020 at 4:52 PM Tyson Hamilton <tyso...@google.com> wrote: >> >>> How did the experiment go? The load status graphs on the executors seem >>> to show no increase in queued jobs [1]. There is a periodic bump every 6h, >>> possibly due to cron firing off a bunch at the same time, that can also be >>> seen by changing the timepsan in [1]. The number of executors on 1/4 of the >>> nodes was also increased so the combination of these things make contention >>> to appear quite low or even non-existent. >>> >>> [1]: https://ci-beam.apache.org/label/beam/load-statistics?type=hour >>> >>> On Mon, Jun 29, 2020 at 9:17 AM Tobiasz Kędzierski < >>> tobiasz.kedzier...@polidea.com> wrote: >>> >>>> 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> >>>> >>>
