Separate history (for easy dashboarding, health stats, etc) and
notification (email to dev@ for postcommits, nothing for precommits) for
pre & post commit targets.
A post commit failure is always a problem to be triaged at high priority,
while a precommit failure is just a natural occurrence.
On Fri, Mar 9, 2018 at 11:33 AM Lukasz Cwik <lc...@google.com> wrote:
> Ken, I'm probably not seeing something but how does using the PreCommit as
> a proxy improve upon just running the post commit via the phrase it already
> supports ('Run Java PostCommit')?
>
> On Fri, Mar 9, 2018 at 11:22 AM, Kenneth Knowles <k...@google.com> wrote:
>
>> Indeed, we've already had the discussion a couple of times and I think
>> the criteria are clearly met. Incremental progress is a good thing and we
>> shouldn't block it.
>>
>> OTOH I see where Romain is coming from and I have a good example that
>> supports a slightly different action. Consider
>> https://github.com/apache/beam/pull/4740 which fixes some errors in how
>> we use dependency mechanisms.
>>
>> This PR is green except that I need to fix some Maven pom slightly more.
>> That is throwaway work. I would love to just not have to do it. But
>> removing the precommit does not actually make the PR OK to merge. It would
>> cause postcommits to fail.
>>
>> We can hope such situations are rare. I think I tend to be hit by this
>> more often than most, as I work with the project build health quite a bit.
>>
>> Here is a proposal to support these things: instead of deleting the job
>> in #4814, move it to not run automatically but only via a phrase. In fact,
>> you could migrate it to be the manually-invoked version of the postcommit
>> job as we've discussed a couple times. Then if someone is working on the
>> build in something like #4740 they can invoke it manually.
>>
>> Kenn
>>
>> On Fri, Mar 9, 2018 at 10:25 AM Lukasz Cwik <lc...@google.com> wrote:
>>
>>> Based upon the criteria that was discussed on the mailing list[1], I
>>> would agree with Kenn about merging PR/4814 (drop Java Maven precommit).
>>>
>>> 1:
>>> https://lists.apache.org/thread.html/7eba5c77bc1a77b5046d915ab59f5f6fc41536c2c84863ad2efb5e99@%3Cdev.beam.apache.org%3E
>>>
>>> On Thu, Mar 8, 2018 at 9:10 PM, Romain Manni-Bucau <
>>> rmannibu...@gmail.com> wrote:
>>>
>>>> Hi Kenneth,
>>>>
>>>> For now maven covers the full needs of beam. If we start to have this
>>>> kind of PR we become dependent of the 2 builds which is what this thread is
>>>> about avoiding so tempted to say it must be a PR drop completely maven or
>>>> nothing as mentionned before.
>>>>
>>>> Le 9 mars 2018 04:48, "Kenneth Knowles" <k...@google.com> a écrit :
>>>>
>>>>> I would like to briefly re-focus this discussion and suggest that we
>>>>> merge https://github.com/apache/beam/pull/4814.
>>>>>
>>>>> The only material objection I've heard is that it means the precommit
>>>>> no longer tests exactly what is built for release. It is a valid concern,
>>>>> but we have mvn postcommit so the coverage is not lost. The benefits of
>>>>> quicker reviews and less Jenkins congestion seem worth it to me.
>>>>>
>>>>> Kenn
>>>>>
>>>>> On Thu, Mar 8, 2018 at 12:33 PM Romain Manni-Bucau <
>>>>> rmannibu...@gmail.com> wrote:
>>>>>
>>>>>> @Luskasz: not sure Im the best to host it since I know more gradle
>>>>>> internals that user interface/ecosystem but happy to help. Will also
>>>>>> require a "sudo" merger for this day to merge fixes asap - guess we can
>>>>>> bypass reviews or have a fast cycle plan for this day to avoid it to be a
>>>>>> week?
>>>>>>
>>>>>> Le 8 mars 2018 21:08, "Kenneth Knowles" <k...@google.com> a écrit :
>>>>>>
>>>>>> @Romain - Idea/IntelliJ is great with Gradle, way better than Maven,
>>>>>> and what I mean is that I have added enough hints that it works OOTB
>>>>>> already. The rest of my instructions are just how you should override
>>>>>> IntelliJ's defaults to have a proper dev env - mostly just about storing
>>>>>> files outside the git clone.
>>>>>>
>>>>>>
>>>>>> Hmm it is always super slow here and not as integrated as maven which
>>>>>> is smooth thanks to the combination of idea build and maven plugin config
>>>>>> read. Import is also faster cause it just reads meta and doesnt run
>>>>>> anything. Hope it is a "a few times" issues at the moment but not yet
>>>>>> sure.
>>>>>>
>>>>>>
>>>>>>
>>>>>> @Łukasz - yea I just mean the ones that I find in my email and
>>>>>> browsing https://builds.apache.org/view/A-D/view/Beam/ which are the
>>>>>> ones you classify as "totally not working". I would love to disable
>>>>>> these.
>>>>>>
>>>>>> On the subject of running things on a pending PR - we should not run
>>>>>> postcommit jobs on PRs. We should make separate (optional) precommit jobs
>>>>>> that run the same build commands. That will give a more clear history and
>>>>>> allow trivial configuration of which jobs deserve alert emails and which
>>>>>> are not a problem. This is easy but I've been waiting to do it after
>>>>>> Gradle
>>>>>> migration.
>>>>>>
>>>>>> On Thu, Mar 8, 2018 at 11:37 AM Lukasz Cwik <lc...@google.com> wrote:
>>>>>>
>>>>>>> Romain, would you like to host/plan/run the Gradle fixit day?
>>>>>>>
>>>>>>> On Thu, Mar 8, 2018 at 11:24 AM, Chamikara Jayalath <
>>>>>>> chamik...@google.com> wrote:
>>>>>>>
>>>>>>>> +1 for the general idea of fixit day/week for Gradle.
>>>>>>>>
>>>>>>>> Agree with what Łukasz said. Some of these performance tests are
>>>>>>>> new and are flaky due to other issues that were discovered during the
>>>>>>>> process of adding the test.
>>>>>>>>
>>>>>>>> I think the high level blocker is updating performance testing
>>>>>>>> framework to use Gradle. This will involve adding Gradle-based logic
>>>>>>>> for
>>>>>>>> invoking PerfKitBenchmaker, for example [1], and updating
>>>>>>>> PerfKitBenchmarker to issue a Gradle command for running the benchmark
>>>>>>>> [2].
>>>>>>>> First task will be to find the work needed here and updating the
>>>>>>>> relevant
>>>>>>>> JIRA [3].
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Cham
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/pom.xml#L79
>>>>>>>> [2]
>>>>>>>> https://github.com/GoogleCloudPlatform/PerfKitBenchmarker/blob/master/perfkitbenchmarker/beam_benchmark_helper.py
>>>>>>>> [3] https://issues.apache.org/jira/browse/BEAM-3251
>>>>>>>>
>>>>>>>> On Thu, Mar 8, 2018 at 3:10 AM Łukasz Gajowy <
>>>>>>>> lukasz.gaj...@gmail.com> wrote:
>>>>>>>>
>>>>>>>>> 2018-03-07 22:29 GMT+01:00 Kenneth Knowles <k...@google.com>:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Based on https://builds.apache.org/view/A-D/view/Beam/ and our
>>>>>>>>>> failure spam level the performance tests are mostly not healthy
>>>>>>>>>> anyhow. So
>>>>>>>>>> is there any high level blocker to switching them or is it just
>>>>>>>>>> someone
>>>>>>>>>> sitting down with each one?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> I thought I could share my point of view here as I am working on
>>>>>>>>> the Performance Test part for a while now. I wouldn't say those are
>>>>>>>>> "mostly
>>>>>>>>> not healthy". The situation is as follows:
>>>>>>>>>
>>>>>>>>> - totally not working: Python, Spark performance tests (don't know
>>>>>>>>> why, I'm not maintaining those tests. Should we disable them?)
>>>>>>>>> - flaky: the recently re-enabled JDBC Performance Test. It's seems
>>>>>>>>> to be flaky mostly due to:
>>>>>>>>> https://issues.apache.org/jira/browse/BEAM-3798
>>>>>>>>> - working well, rarely failing: AvroIO, TextIO, Compressed Text,
>>>>>>>>> TFRecordIO
>>>>>>>>>
>>>>>>>>> Also, some test failures are caused due to pending PRs (we
>>>>>>>>> sometimes run concrete tests to check if PRs won't break them). This
>>>>>>>>> also
>>>>>>>>> causes failures sometimes.
>>>>>>>>>
>>>>>>>>> I can help with switching the Performance tests to Gradle as this
>>>>>>>>> part seems to be free to take.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Łukasz
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>
>
