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. > >> > >> > > > >> >> >> >> > >> > >> > > > >> >> >> > >> > >> > > > >> >> > > >> > >> > > > >> >> > > >> > >> > > > >> >> > >> > >> > > > >> > >> > >> > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > > > >> > > > >> > > >> >
