impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philip Zeyliger <phi...@cloudera.com>
Subject Re: Impala 3.x (and 2.x)
Date Fri, 19 Jan 2018 21:59:36 GMT
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 <philip@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 philz@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://philz@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.behm@cloudera.com>
> wrote:
>
>> +1
>>
>> On Tue, Jan 16, 2018 at 5:27 PM, Taras Bobrovytsky <tarasbob@apache.org>
>> wrote:
>>
>> > +1
>> >
>> > On Tue, Jan 16, 2018 at 1:34 PM, Lars Volker <lv@cloudera.com> wrote:
>> >
>> > > +1
>> > >
>> > > On Tue, Jan 16, 2018 at 1:29 PM, Philip Zeyliger <philip@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 <
>> philip@cloudera.com>
>> > > > wrote:
>> > > >
>> > > > >
>> > > > >
>> > > > > On Fri, Jan 12, 2018 at 4:04 PM, Jim Apple <jbapple@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 <
>> > philip@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 <
>> jbapple@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
<
>> > > > philip@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
<
>> > > jbapple@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 <
>> > > > >> philip@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 <
>> > > > jbapple@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.
>> > > > >> >> >> >>
>> > > > >> >> >>
>> > > > >> >> >
>> > > > >> >> >
>> > > > >> >>
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message