I looked at the Jenkins job configuration and it looked good to me. Thank you for working on this.
Cheers, Lars On Tue, Jan 23, 2018 at 4:07 PM, Philip Zeyliger <phi...@cloudera.com> wrote: > Hi folks! > > As an update here: I've built > https://jenkins.impala.io/job/cherrypick-2.x-and-test/ as a test bed for > how this could work. (Because the cherrypick tool at review 9045 is still > pending GVO, I just hard-coded a single cherrypick.) Whenever "master" > changes, that Jenkins jobs would use a cherrypick tool to cherrypick things > into refs/sandbox/impala-public-jenkins/2.x-staging. Then, it would run > parallel-all-tests. If parallel-all-tests passes, all the cherrypicks are > promoted to asf-gerrit/2.x. > > I thought about doing one test run per commit, but I think that's very > liable to get stuck. I think we occasionally have commits that introduce a > test failure, followed by a commit that fixes it. The commit that > introduced the test failure would never pass the gate, and we'd get stuck. > (There's obviously the question of how it passed the original gate, but > that sometimes happens due to races and environmental factors.) > > Please let me know if you've got any concerns! > > -- Philip > > > On Fri, Jan 19, 2018 at 4:17 PM, Jim Apple <jbap...@cloudera.com> wrote: > > > We could commit first and then GVO -- that way if a commit broke > > something (in a reproducible way), we would at least know which commit > > did it. In this proposal, we don't have to wait for GVO N to finish > > before committing patch N+1. This is a sort of fast-path-slow-path > > idea, but fixing the slow path while keeping 2.x stable could be > > tricky. > > > > On Fri, Jan 19, 2018 at 3:53 PM, Dimitris Tsirogiannis > > <dtsirogian...@cloudera.com> wrote: > > > My ideal and more conservative policy would be: > > > - If conflicts, review. > > > - Always GVO. > > > > > > Dimitris > > > > > > On Fri, Jan 19, 2018 at 2:13 PM, Taras Bobrovytsky < > taras...@apache.org> > > > wrote: > > > > > >> I like your policy suggestion. If there are no conflicts, then we can > > push > > >> from master to 2.x without any tests or reviews. If there are > conflicts, > > >> then a review AND a GVO test run are required. > > >> > > >> We can at least start off with this policy and then change it if it is > > not > > >> working well for some reason. > > >> > > >> On Fri, Jan 19, 2018 at 1:59 PM, Philip Zeyliger <phi...@cloudera.com > > > > >> wrote: > > >> > > >> > An update here! > > >> > > > >> > I'm pretty close to pushing the first master-only change (the very > > >> exciting > > >> > 1-liner at https://gerrit.cloudera.org/#/c/9044/ that bumps > > versions). > > >> > After that, I'll be cherrypicking things into 2.x. > > >> > > > >> > We need a policy, I think, on reviewing these cherry-picks. The most > > >> > heavy-weight would be to do Gerrit and run-tests-before-merge for > > every > > >> > change. The least heavy would be to review only when the > cherry-picks > > >> > aren't clean. Are people comfortable with the less heavy policy? > > >> > > > >> > Specifically, I propose that clean cherrypicks from master to 2.x > can > > be > > >> > pushed without additional review. > > >> > > > >> > Thanks! > > >> > > > >> > -- Philip > > >> > > > >> > On Wed, Jan 17, 2018 at 9:57 AM, Philip Zeyliger < > phi...@cloudera.com > > > > > >> > wrote: > > >> > > > >> > > It sounds like we've reached consensus, so I'm starting to > maneuver > > >> this > > >> > > along. Please don't hesitate if you've got concerns. You can > follow > > >> along > > >> > > at https://issues.apache.org/jira/browse/IMPALA-6410. > > >> > > > > >> > > The first two reviews are out at: > > >> > > > > >> > > remote: http://gerrit.cloudera.org:8080/9044 Bumping version to > > 3.0. > > >> > > remote: http://gerrit.cloudera.org:8080/9045 IMPALA-6410: Tool > to > > >> > > cherrypick changes across branches. > > >> > > > > >> > > I've created the branches on Gerrit and Apache as follows: > > >> > > > > >> > > # This created the branch on gerrit.cloudera.org > > >> > > $ssh -p 29418 ph...@gerrit.cloudera.org gerrit create-branch > > >> Impala-ASF > > >> > > 2.x master > > >> > > > > >> > > # This created the branch on the Apache git server: > > >> > > $git push apache asf-gerrit/2.x:refs/heads/2.x > > >> > > Username for 'https://git-wip-us.apache.org': philz > > >> > > Password for 'https://ph...@git-wip-us.apache.org': > > >> > > Total 0 (delta 0), reused 0 (delta 0) > > >> > > To https://git-wip-us.apache.org/repos/asf/impala.git > > >> > > * [new branch] asf-gerrit/2.x -> 2.x > > >> > > > > >> > > # Double-check: > > >> > > $git-ls-remote apache | grep 2.x; git-ls-remote asf-gerrit | grep > > 2.x > > >> > > 6cc76d72016b8d5672676e8e8a979b0807803ad9 refs/heads/2.x > > >> > > 6cc76d72016b8d5672676e8e8a979b0807803ad9 refs/heads/2.x > > >> > > > > >> > > # push_to_asf learned of the branch "magically" > > >> > > $bin/push_to_asf.py > > >> > > INFO:root:Fetching from remote 'asf-gerrit'... > > >> > > INFO:root:done > > >> > > Branch '2.x': up to date > > >> > > Branch 'asf-site': up to date > > >> > > Branch 'branch-2.10.0': found on Apache but not in gerrit > > >> > > Branch 'branch-2.11.0': found on Apache but not in gerrit > > >> > > Branch 'branch-2.7.0': found on Apache but not in gerrit > > >> > > Branch 'branch-2.8.0': found on Apache but not in gerrit > > >> > > Branch 'branch-2.9.0': found on Apache but not in gerrit > > >> > > Branch 'hadoop-next': up to date > > >> > > Branch 'master': up to date > > >> > > > > >> > > -- Philip > > >> > > > > >> > > On Tue, Jan 16, 2018 at 5:29 PM, Alexander Behm < > > >> alex.b...@cloudera.com> > > >> > > wrote: > > >> > > > > >> > >> +1 > > >> > >> > > >> > >> On Tue, Jan 16, 2018 at 5:27 PM, Taras Bobrovytsky < > > >> taras...@apache.org > > >> > > > > >> > >> wrote: > > >> > >> > > >> > >> > +1 > > >> > >> > > > >> > >> > On Tue, Jan 16, 2018 at 1:34 PM, Lars Volker <l...@cloudera.com> > > >> wrote: > > >> > >> > > > >> > >> > > +1 > > >> > >> > > > > >> > >> > > On Tue, Jan 16, 2018 at 1:29 PM, Philip Zeyliger < > > >> > phi...@cloudera.com > > >> > >> > > > >> > >> > > wrote: > > >> > >> > > > > >> > >> > > > Hi folks! > > >> > >> > > > > > >> > >> > > > It sounds like there haven't been objections to having > > master be > > >> > >> "3.0" > > >> > >> > > and > > >> > >> > > > introducing a 2.x branch. Would folks be alright if I > started > > >> > making > > >> > >> > > > changes in that direction? > > >> > >> > > > > > >> > >> > > > Thanks! > > >> > >> > > > > > >> > >> > > > -- Philip > > >> > >> > > > > > >> > >> > > > On Fri, Jan 12, 2018 at 4:10 PM, Philip Zeyliger < > > >> > >> phi...@cloudera.com> > > >> > >> > > > wrote: > > >> > >> > > > > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > On Fri, Jan 12, 2018 at 4:04 PM, Jim Apple < > > >> > jbap...@cloudera.com> > > >> > >> > > wrote: > > >> > >> > > > > > > >> > >> > > > >> This makes sense to me. > > >> > >> > > > >> > > >> > >> > > > >> In this mode, for 2.x-only changes and for changes on > 3.0 > > >> that > > >> > >> don't > > >> > >> > > > >> apply cleanly, there will be a manual way to do the step > > >> > labelled > > >> > >> > "1. > > >> > >> > > > >> Cherrypick tool", and that way is the same way we send > > >> patches > > >> > >> for > > >> > >> > > > >> review now, but pushing to HEAD:refs/for/2.x rather than > > >> > >> > > > >> HEAD:refs/for/master, yes? > > >> > >> > > > >> > > >> > >> > > > > > > >> > >> > > > > Exactly. So, non-clean cherrypicks or 2.x-only changes go > > >> > through > > >> > >> > > review > > >> > >> > > > > on Gerrit, but we give an implicit review pass to clean > > >> > >> cherrypicks. > > >> > >> > > > > > > >> > >> > > > > We could have the cherrypick tool between gerrit/master > and > > >> > >> > gerrit/2.x > > >> > >> > > do > > >> > >> > > > > the cherrypicks and run the tests on Jenkins. Do you > think > > >> > that's > > >> > >> > > > > preferable? > > >> > >> > > > > > > >> > >> > > > > -- Philip > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > >> > > >> > >> > > > >> On Fri, Jan 12, 2018 at 3:57 PM, Philip Zeyliger < > > >> > >> > phi...@cloudera.com > > >> > >> > > > > > >> > >> > > > >> wrote: > > >> > >> > > > >> > Picture: > > >> > >> > > > >> > https://gist.github.com/philz/ > > >> 323c8b4cb411dc12eb7231d922c195 > > >> > >> > > > >> 1f#file-impala-branch-image-pdf > > >> > >> > > > >> > > > >> > >> > > > >> > > > >> > >> > > > >> > On Fri, Jan 12, 2018 at 3:47 PM, Jim Apple < > > >> > >> jbap...@cloudera.com> > > >> > >> > > > >> wrote: > > >> > >> > > > >> > > > >> > >> > > > >> >> Often, this list seems to filter out images. Could > you > > >> post > > >> > it > > >> > >> > and > > >> > >> > > > >> send a > > >> > >> > > > >> >> link? > > >> > >> > > > >> >> > > >> > >> > > > >> >> Thanks for taking this on, Phil! > > >> > >> > > > >> >> > > >> > >> > > > >> >> On Fri, Jan 12, 2018 at 3:15 PM, Philip Zeyliger < > > >> > >> > > > phi...@cloudera.com> > > >> > >> > > > >> >> wrote: > > >> > >> > > > >> >> > > >> > >> > > > >> >> > I think most patches go to Gerrit branch 'master', > > which > > >> > >> > happens > > >> > >> > > to > > >> > >> > > > >> >> > identify itself as 3.0. (Or 3.x?). > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > Here's a picture: > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > [image: Inline image 1] > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > With this, every time "cherrypick_and_push_to_asf. > > py" > > >> is > > >> > >> run, > > >> > >> > it > > >> > >> > > > >> would > > >> > >> > > > >> >> > first offer to cherrypick changes between master > and > > >> 2.x. > > >> > >> Then, > > >> > >> > > it > > >> > >> > > > >> would > > >> > >> > > > >> >> > offer push those cherrypicks to gerrit/2.x. After > > that, > > >> it > > >> > >> > > > continues > > >> > >> > > > >> on > > >> > >> > > > >> >> as > > >> > >> > > > >> >> > before and offers to push changes to ASF. I think > > this > > >> > >> > maintains > > >> > >> > > > the > > >> > >> > > > >> >> > invariant that pushing to ASF is only done with a > > human > > >> > >> > trigger. > > >> > >> > > > (We > > >> > >> > > > >> >> could > > >> > >> > > > >> >> > also have step 1 be done by a Jenkins robot, since > > it's > > >> > >> between > > >> > >> > > > >> Gerrit > > >> > >> > > > >> >> and > > >> > >> > > > >> >> > Gerrit.) > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > I looked at the How to Release page, and the main > > >> > difference > > >> > >> > > would > > >> > >> > > > be > > >> > >> > > > >> >> > that, for a 2.x release, the $COMMIT_HASH_YOU_CHOSE > > >> would > > >> > >> come > > >> > >> > > from > > >> > >> > > > >> the > > >> > >> > > > >> >> 2.x > > >> > >> > > > >> >> > branch, as would any cherrypicks. > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > Does this match what you're thinking? > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > Thanks! > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > -- Philip > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > On Fri, Jan 12, 2018 at 11:07 AM, Jim Apple < > > >> > >> > > jbap...@cloudera.com> > > >> > >> > > > >> >> wrote: > > >> > >> > > > >> >> > > > >> > >> > > > >> >> >> Which gerrit branch were you thinking most patches > > >> would > > >> > go > > >> > >> > to? > > >> > >> > > > >> >> >> > > >> > >> > > > >> >> >> If they go to 3.0, then push_to_asf.py would have > > to be > > >> > >> > amended > > >> > >> > > to > > >> > >> > > > >> >> >> push to gerrit, bypassing code review. I think > > that's > > >> > >> > possible, > > >> > >> > > > but > > >> > >> > > > >> >> >> I'm not 100%. > > >> > >> > > > >> >> >> > > >> > >> > > > >> >> >> There is also security to think about, since the > > >> > >> > push_to_asf.py > > >> > >> > > > >> users > > >> > >> > > > >> >> >> can push a few commits at a time, including ones > > they > > >> > >> didn't > > >> > >> > > > author > > >> > >> > > > >> or > > >> > >> > > > >> >> >> review. > > >> > >> > > > >> >> >> > > >> > >> > > > >> >> >> We'll also want to clarify > > >> > >> > > > >> >> >> https://cwiki.apache.org/ > > >> confluence/display/IMPALA/How+ > > >> > >> > > to+Release > > >> > >> > > > >> and > > >> > >> > > > >> >> >> keep it consistent with the git & gerrit statuses > > quo. > > >> > >> > > > >> >> >> > > >> > >> > > > >> >> >> On Fri, Jan 12, 2018 at 10:52 AM, Philip Zeyliger > < > > >> > >> > > > >> phi...@cloudera.com> > > >> > >> > > > >> >> >> wrote: > > >> > >> > > > >> >> >> > Hi! > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> >> Should we start tagging all candidates with a > > common > > >> > >> label, > > >> > >> > > > e.g. > > >> > >> > > > >> >> >> > include-in-v3? > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > I agree with Lars's suggestion for tagging JIRAs > > with > > >> > >> > > > >> include-in-v3. > > >> > >> > > > >> >> >> I've > > >> > >> > > > >> >> >> > done so, and the relevant query is > > >> > >> > > > >> >> >> > https://issues.apache.org/ > > >> > jira/issues/?jql=labels%20%3D% > > >> > >> > 20in > > >> > >> > > > >> >> >> clude-in-v3%20and%20project%3Dimpala > > >> > >> > > > >> >> >> > . > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> >> What sort of process were you thinking of for > the > > >> > >> > automation? > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > I think amending push_to_asf.py, as you suggest, > > is a > > >> > >> great > > >> > >> > > > idea. > > >> > >> > > > >> I > > >> > >> > > > >> >> >> think > > >> > >> > > > >> >> >> > we have a string ("not for 2.x") which can be > > used in > > >> > >> commit > > >> > >> > > > >> messages > > >> > >> > > > >> >> to > > >> > >> > > > >> >> >> > discourage the cherrypick for the changes we > want > > to > > >> be > > >> > >> > > > exclusive > > >> > >> > > > >> >> until > > >> > >> > > > >> >> >> we > > >> > >> > > > >> >> >> > want to change the defaults in the other > > direction. > > >> > >> (I.e., > > >> > >> > > right > > >> > >> > > > >> now > > >> > >> > > > >> >> the > > >> > >> > > > >> >> >> > string is "not for 2.x", but at some point the > > string > > >> > >> may be > > >> > >> > > > >> "should > > >> > >> > > > >> >> be > > >> > >> > > > >> >> >> > cherrypicked to 2.x".) > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > I do think that we want to create a gerrit > branch > > to > > >> > >> allow > > >> > >> > > > >> 2.x-only > > >> > >> > > > >> >> >> changes > > >> > >> > > > >> >> >> > to be reviewed in the straight-forward fashion. > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > -- Philip > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> > On Thu, Jan 11, 2018 at 9:31 AM, Jim Apple < > > >> > >> > > > jbap...@cloudera.com> > > >> > >> > > > >> >> >> wrote: > > >> > >> > > > >> >> >> > > > >> > >> > > > >> >> >> >> I'm on-board with all of this. (I also would be > > OK > > >> > >> delaying > > >> > >> > > > 3.0, > > >> > >> > > > >> if > > >> > >> > > > >> >> >> >> that were the consensus). > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> There is one issue in here I think we should > dive > > >> > into: > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> > Both master and 2.x would be active, and, at > > least > > >> > for > > >> > >> > the > > >> > >> > > > >> >> beginning, > > >> > >> > > > >> >> >> >> > changes would automatically be pulled into > the > > 2.x > > >> > >> line, > > >> > >> > > > unless > > >> > >> > > > >> >> >> >> explicitly > > >> > >> > > > >> >> >> >> > blacklisted. > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> What sort of process were you thinking of for > the > > >> > >> > automation? > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> Some context, starting from what we all likely > > >> already > > >> > >> > know: > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> The bulk of the code review and pre-merge > testing > > >> > >> results > > >> > >> > are > > >> > >> > > > >> >> recorded > > >> > >> > > > >> >> >> >> in gerrit. Once the pre-merge testing passes, a > > >> patch > > >> > is > > >> > >> > > > >> >> cherry-picked > > >> > >> > > > >> >> >> >> to the git repo hosted with gerrit. To get the > > patch > > >> > to > > >> > >> the > > >> > >> > > > >> Impala > > >> > >> > > > >> >> git > > >> > >> > > > >> >> >> >> repo hosted by the ASF, bin/push_to_asf.py is > run > > >> by a > > >> > >> > human > > >> > >> > > > who > > >> > >> > > > >> >> >> >> supplies his or her ASF credentials. That > script > > >> > copies > > >> > >> the > > >> > >> > > > >> commit to > > >> > >> > > > >> >> >> >> the ASF git repo. > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> Often, 2-3 commits will pile up in gerrit > before > > >> some > > >> > >> > > committer > > >> > >> > > > >> runs > > >> > >> > > > >> >> >> >> that script and pushes them to ASF. > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> We could edit that script (bin/push_to_asf.py) > to > > >> help > > >> > >> with > > >> > >> > > the > > >> > >> > > > >> >> cherry > > >> > >> > > > >> >> >> >> picks, so that each time a commit is made, the > > >> > committer > > >> > >> > must > > >> > >> > > > say > > >> > >> > > > >> >> >> >> whether the commit goes in 2.x, 3.0, or both, > but > > >> the > > >> > >> > commits > > >> > >> > > > are > > >> > >> > > > >> >> >> >> often made by people who didn't author the > > patches, > > >> so > > >> > >> they > > >> > >> > > may > > >> > >> > > > >> not > > >> > >> > > > >> >> be > > >> > >> > > > >> >> >> >> sure which branch to go in. > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> >> Additionally, gerrit code review is intimately > > tied > > >> to > > >> > >> the > > >> > >> > > git > > >> > >> > > > >> repo. > > >> > >> > > > >> >> >> >> Gerrit runs a git repo under-the-hood, and I > > believe > > >> > >> that > > >> > >> > the > > >> > >> > > > >> branch > > >> > >> > > > >> >> >> >> on gerrit's git that changes are cherry-picked > to > > >> > after > > >> > >> > > > pre-merge > > >> > >> > > > >> >> >> >> testing is identical to the Impala git repo > > hosted > > >> by > > >> > >> the > > >> > >> > > ASF - > > >> > >> > > > >> down > > >> > >> > > > >> >> >> >> to the hashes, even. If we think 2.x and 3.0 > will > > >> > >> diverge > > >> > >> > > > enough > > >> > >> > > > >> that > > >> > >> > > > >> >> >> >> we'll want different code reviews for different > > >> > >> branches, > > >> > >> > > then > > >> > >> > > > we > > >> > >> > > > >> >> >> >> might want two different branches on gerrit, > too. > > >> > >> > > > >> >> >> >> > > >> > >> > > > >> >> >> > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > > > >> > >> > > > >> >> > > >> > >> > > > >> > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > > > > >> > > > > >> > > > >> > > >
