On Wed, Oct 25, 2017 at 6:51 PM, Robert Bradshaw < [email protected]> wrote: > > These are just some ideas; I honestly don't know. I think my upper limit > for default precommit feedback is probably 30 minutes, and even there I am > not very happy. > > > We should be able to get a good signal in less than 30 minutes. Lint, smoke > tests, etc. I think there is a more extensive set of tests that should pass > before merging. >
Yea, definitely. Digging into this in particular because I think it is quite promising as a way to reduce build congestion. It looks like `mvn clean test -Dfindbugs.skip` would finish in 10 minutes if we fixed it. Currently it looks like some modules don't work under this invocation, probably requiring some state to be established by prior bits run by `verify`. If we iterated on that until review was done, then 30-90 minutes to prevent breakage would be fine, especially since a failure in that longer slower build is probably a signal to add a quicker repro unit test. Kenn > > Also it might not hurt to audit our tests and see if there are a small > number that take a disproportionate amount of time. > > Any other suggestions? > > Kenn > > On Mon, Oct 23, 2017 at 3:29 PM, Kenneth Knowles <[email protected]> wrote: > > > I want to wait and get some green from Jenkins running against the HEAD > > groovy scripts to confirm. I haven't sat at my desk long enough to see a > > full `mvn -P release clean verify` yet. > > > > On Mon, Oct 23, 2017 at 3:27 PM, Kenneth Knowles <[email protected]> wrote: > > > >> I was easily able to reproduce all the sorts of failures (and some > more!) > >> > >> Here are some things that now work that didn't work, or didn't work > >> correctly, before > >> > >> - mvn apache-rat:check > >> - mvn -P release apache-rat:check > >> - mvn -P release -f somewhere/else/pom.xml apache-rat:check > >> > >> It turns out we had these issues: > >> > >> - items in .gitignore that our RAT config did not ignore > >> - sub-modules actually *did* inherit the RAT config, depending on the > >> command you run > >> - paths in the RAT exclude were relative to current dir > >> - paths in our RAT exclude that aren't even part of our codebase or > >> generated targets, but just things that Jenkins dropped in the > workspace, > >> and we were cloning directly into the workspace > >> > >> I think with the new Jenkins config and the new pom.xml things should > >> work well. > >> > >> Incidentally, Valentyn, changes to Jenkins job DSL groovy scripts do not > >> take effect from being merged but only when the seed job next runs. > (TODO: > >> fix this). I am still trying to get the relevant Jenkins UI pages to > load > >> to get your change incorporated into the live scripts, which were > regressed > >> by being run from an old PR (TODO: fix the fact that this can happen). > >> > >> Kenn > >> > >> On Mon, Oct 23, 2017 at 2:21 PM, Valentyn Tymofieiev < > >> [email protected]> wrote: > >> > >>> Thanks a lot! > >>> > >>> Kenn, Were you able to reproduce RAT failures and test the fix locally? > I > >>> think "mvn clean verify -Prelease" still passes for me. > >>> > >>> Timeout for the test suite has been increased in > >>> https://github.com/apache/ > >>> beam/pull/4028 <https://github.com/apache/beam/pull/4028>. > >>> > >>> On Mon, Oct 23, 2017 at 2:10 PM, Kenneth Knowles > <[email protected] > >>> > > >>> wrote: > >>> > >>> > Wrong link - https://github.com/apache/beam/pull/4027 > >>> > > >>> > On Mon, Oct 23, 2017 at 2:10 PM, Kenneth Knowles <[email protected]> > >>> wrote: > >>> > > >>> > > Yea, root cause is the config bug I described. Proposed fix at > >>> > > https://github.com/apache/beam/pull/4019/files. I'm working with > >>> infra > >>> > to > >>> > > sort out other build issues that are probably not related. > >>> > > > >>> > > On Mon, Oct 23, 2017 at 1:53 PM, Lukasz Cwik > >>> <[email protected]> > >>> > > wrote: > >>> > > > >>> > >> The build breakage I outlined is being tracked in > >>> > >> https://issues.apache.org/jira/browse/BEAM-3092 > >>> > >> > >>> > >> On Mon, Oct 23, 2017 at 11:54 AM, Lukasz Cwik <[email protected]> > >>> wrote: > >>> > >> > >>> > >> > Another breaking change was caused by > https://github.com/apache/ > >>> > >> > beam/commit/241d3cedd5a8fd3f360b8ec2f3a8ef5001cbca98 because it > >>> > changed > >>> > >> > the build layout on the Jenkins server and our RAT rules > expected > >>> to > >>> > >> apply > >>> > >> > from a root directory. I pinged Kenneth Knowles about it and he > >>> said > >>> > he > >>> > >> was > >>> > >> > taking a look. > >>> > >> > > >>> > >> > On Mon, Oct 23, 2017 at 11:53 AM, Raghu Angadi > >>> > >> <[email protected] > >>> > >> > > wrote: > >>> > >> > > >>> > >> >> Regd (1) : > >>> > >> >> > >>> > >> >> [4] did have have a file without Apache Licence. It was fixed > the > >>> > next > >>> > >> >> day ( > >>> > >> >> commit > >>> > >> >> <https://github.com/apache/beam/commit/249da9b8a1e86d0fe4c3d > >>> > >> >> c7b83032ad38c3dcac0#diff-26b77e086ff8292ef54f12b22b7b767a>), > >>> > >> >> thanks to Ken Knowles who pinged me about it. > >>> > >> >> > >>> > >> >> On Mon, Oct 23, 2017 at 11:45 AM, Valentyn Tymofieiev < > >>> > >> >> [email protected]> > >>> > >> >> wrote: > >>> > >> >> > >>> > >> >> > Hi Beam-Dev, > >>> > >> >> > > >>> > >> >> > It's been >5 days since the last successful run of a > >>> > >> >> > beam_PreCommit_Java_MavenInstall build[1] and >4 days since > >>> last > >>> > >> >> > successful run of beam_PreCommit_Java_MavenInstall[2]. > >>> > >> >> > > >>> > >> >> > Looking at build logs I see following problems. > >>> > >> >> > > >>> > >> >> > 1. After October 17, postcommit builds started to fail with > >>> > >> >> > > >>> > >> >> > Failed to execute goal org.apache.rat:apache-rat-plug > >>> in:0.12:check > >>> > >> >> > (default) on project beam-parent: Too many files with > >>> unapproved > >>> > >> >> license: 1 > >>> > >> >> > See RAT report in: /home/jenkins/jenkins-slave/wo > >>> > >> >> > rkspace/beam_PostCommit_Java_MavenInstall/target/beam-paren > >>> > >> >> > t-2.3.0-SNAPSHOT.rat > >>> > >> >> > > >>> > >> >> > The earliest build that I see this error is Postcommit #5052 > >>> [3]. > >>> > >> >> > > >>> > >> >> > This makes me suspect [4] or [5] as a breaking change, since > >>> they > >>> > >> change > >>> > >> >> > pom files. > >>> > >> >> > > >>> > >> >> > Questions: > >>> > >> >> > - Is there a way we can reproduce this failure locally? mvn > >>> clean > >>> > >> verify > >>> > >> >> > passes locally for me. > >>> > >> >> > - Is there a way we can see the See RAT report mentioned in > the > >>> > error > >>> > >> >> > log? > >>> > >> >> > > >>> > >> >> > 2. Prior to onset of #1 Java Precommit builds no longer > >>> complete > >>> > >> within > >>> > >> >> > allotted 150 min time. Looking at [6-8] it seems the build > >>> makes > >>> > >> >> consistent > >>> > >> >> > progress, but just does not finish on time. We can also see > >>> several > >>> > >> >> recent > >>> > >> >> > successful builds with execution time very close to time out > >>> > [9-11]. > >>> > >> >> > > >>> > >> >> > I'd like to propose to increase time limit for Java precommit > >>> test > >>> > >> suite > >>> > >> >> > from 2.5 to 4 hours. 4 hours is long time. I agree that we > >>> should > >>> > >> >> > definitely try to reduce the test execution time, and reduce > >>> > >> flakiness. > >>> > >> >> > However we need the tests at least pass for now. If we write > >>> off > >>> > >> failed > >>> > >> >> > test suites as 'flakes' and merge PRs without having a green > >>> test > >>> > >> >> signal, > >>> > >> >> > we will have to spend more time tracing breakages such as #1. > >>> > >> >> > > >>> > >> >> > Thoughts? > >>> > >> >> > > >>> > >> >> > Thanks, > >>> > >> >> > Valentyn > >>> > >> >> > > >>> > >> >> > [1] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > ll/ > >>> > >> >> > [2] https://builds.apache.org/job/ > >>> beam_PostCommit_Java_MavenInst > >>> > all/ > >>> > >> >> > [3] https://builds.apache.org/job/ > >>> beam_PostCommit_Java_MavenInst > >>> > >> >> > all/5052/changes > >>> > >> >> > > >>> > >> >> > [4] https://github.com/apache/beam/commit/d745cc9d8cc1735d3b > >>> > >> >> > c3c67ba3e2617cb7f11a8c > >>> > >> >> > [5] https://github.com/apache/beam > >>> /commit/0d8ab6cbbc762dd9f9be1b > >>> > >> >> > 3e9a26b6c9d0bb6dc3 > >>> > >> >> > > >>> > >> >> > [6] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > >> >> ll/15222/ > >>> > >> >> > [7] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > >> >> ll/15195/ > >>> > >> >> > [8] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > >> >> ll/15220/ > >>> > >> >> > > >>> > >> >> > [9] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > >> >> ll/15009/ > >>> > >> >> > [10] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > >> >> ll/15068/ > >>> > >> >> > [11] https://builds.apache.org/job/ > >>> beam_PreCommit_Java_MavenInsta > >>> > >> >> ll/15016/ > >>> > >> >> > > >>> > >> >> > > >>> > >> >> > >>> > >> > > >>> > >> > > >>> > >> > >>> > > > >>> > > > >>> > > >>> > >> > >> > > >
