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 Wed, 24 Jan 2018 00:07:19 GMT
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 <jbapple@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
> <dtsirogiannis@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 <tarasbob@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 <philip@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 <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