hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stack <st...@duboce.net>
Subject Re: [DISCUSSION] Switching from RTC to CTR
Date Mon, 17 Aug 2015 21:21:50 GMT
On Sun, Aug 16, 2015 at 11:40 AM, Andrew Purtell <apurtell@apache.org>
wrote:

> > As Stack mentioned, having the communal norm of reviews means that folks
> are more likely to see more of the code.
>
> I don't get it, because that norm can exist equally under CTR as RTC.
>
>
How? If strictly CTR, yes, but I've been reading CTR in this discussion as
CTOR (Commit, Then, Optionally Review). Correct me if I have it wrong.



> Let me repeat a question I had earlier that nobody has responded to: What
> would be the practical difference/outcome of a committer immediately
> checking in something today without giving time for review versus doing so
> if we said "CTR" or "CTR after one week" *AND* "good committer practice is
> to get a code review and +1 before commit"? Practically, I mean.



Probably none -- former would be 'abnormal' so would likely trigger peer
interjection while the latter would be the norm and more likely to be let
slide.


In both
> cases we'd revert it and have a discussion. Would the lack of attention to
> good practice be more actionable by the PMC because in the first case it is
> a rule violation and in the latter it's just inattention to community
> norms. Is that it? We need rules? (And is that "just because", or is there
> really a lack of trust?)
>
>
Yes. We need a few rules and a bit of process. Having a simple set makes
distributed dev by a bunch of peers run smoother.

Above in this thread are a few arguments on how RTC does not have to be
interpreted as lack of trust.



> Anyway, I like how in this discussion members of the PMC have expressed
> some room for committers to make minor commits on their good judgement
> without _necessarily_ getting a review first in order to make progress. My
> motivation here is we are a community of busy people and some things just
> can't get the level of interest as others. As an RM, things like build
> fixes to unblock release candidates of older branches come to mind. I also
> like that we have plenty of room for experimental work on branches with
> scrutiny coming later at branch commit time with a very reasonable bar of 3
> +1s inviting necessary scrutiny. I still think a policy of "one week for
> review, then proceed as CTR" would be useful for a variety of reasons but
> don't see clear support for that here. After I collect a few instances of
> recent anecdotal evidence supporting it I may revive this discussion.
>
>
Ok.
St.Ack





>
> On Fri, Aug 14, 2015 at 9:00 PM, Sean Busbey <busbey@cloudera.com> wrote:
>
> > My apologies if this thread has run its course and I'm showing up late to
> > rehash things.
> >
> > Here's a short list of the reasons I like RTC:
> >
> > 1) Number One With A Bullet: It puts committers and
> > non-committer-contributors on closer to equal footing. If I'm
> participating
> > in the project and I haven't been blessed with a commit bit, what am I
> > supposed to do after my week of having my patch sit around?
> >
> > 2) Community interaction. As Stack mentioned, having the communal norm of
> > reviews means that folks are more likely to see more of the code.
> >
> > 3) Everyone has a bad day. I totally identify with committership being a
> > sign of "I trust you" to project participants. But everyone has one of
> > those days where you're in a rush either because of work or life. Having
> > even a cursory additional set of eyes on things markedly increases the
> > quality of the overall code base over a long enough time line (at least
> in
> > my experience contributing to open source projects). So for me, the trust
> > is largely "to follow the rules" and "to provide feedback in reviews".
> >
> > If we end up with some part of the code that isn't getting reviews, I'd
> > rather the PMC fix that problem instead of backslide on the three points
> > above.
> >
> > That we don't have this problem right now is wonderful. I have been in /
> am
> > currently in some projects where the community is very near end-of-life
> by
> > the ASF's definition of 3 folks to approve a release. My observation of
> > those communities has the CTR ones much more a loose collection of people
> > scratching their own itch. When an ASF project gets to that point, what
> the
> > advantage over just going to the attic and keeping independent forks for
> > those few remaining folks?
> >
> > I kind of view this like the ASF policy on only distributing PMC approved
> > releases. The advice from the foundation for folks who don't like the
> > limitation of waiting for a release is "make more releases." Similarly, I
> > think the problem of reviewer bandwidth is solved with "make more
> > committers."
> >
> > I don't want to hijack this thread, but maybe we could have another one
> > about expectations for committership and ways the PMC could help get more
> > folks on the path?
> >
> >
> > On Sun, Aug 2, 2015 at 1:59 PM, Andrew Purtell <andrew.purtell@gmail.com
> >
> > wrote:
> >
> > > Agreed, committers should be spending more time reviewing others' work.
> > We
> > > can ask. It may happen. It may work for a short while. It may not.
> Shrug.
> > > People will do what they want.
> > >
> > > I'm looking to make one substantial change that will allow committers
> to
> > > make progress even if there's nobody else around or interested for one
> > > week. It happens sometimes. I've already talked about my concerns on
> > > assuming a certain level of available volunteer bandwidth. Let me just
> > say
> > > that things are great now, it's fantastic.
> > >
> > > We are pretty much on the honor system already. I don't buy the
> argument
> > > we can't trust that CTR or CTR after one week can work because
> > committers,
> > > even if asked to customarily get a review before commit, may decide to
> > > check in unreviewed untested destabilizing changes. At least, In that
> > case
> > > I'd argue we have a different problem. If you go back to my original
> > mail,
> > > I do say we shouldn't undertake any change if we as PMC are unwilling
> to
> > > revoke the commit bit from committers who sadly demonstrate themselves
> > > unworthy of trust through checkins of toxic waste without review. Merit
> > > _can_ be un-earned. Commits can be reverted. Just because something is
> > > checked in does not mean it will go out in a release. We could put a
> > > nightly suite of regression tests in place. Both proposals I have made,
> > > especially the latter, are not a binary release of any attempt at
> quality
> > > control. We would have all of the consensus expectations of good
> > committer
> > > behavior still in place.
> > >
> > > Let's assume someone checks in something without getting a review
> today.
> > > What would happen? Someone else would revert it and we'd have a
> > discussion.
> > > If we were operating under CTR-after-one-week (or plain CTR) but with
> > > documented expectations that someone get a review first, what changes,
> in
> > > terms of quality control and project discipline? Maybe the difference
> is
> > we
> > > could more easily justify revoking commit privileges? Maybe. I think
> > every
> > > discussion like that would be strictly case by case though, a
> > conversation
> > > between that person and the PMC, justification for revoking committer
> > > status wouldn't be rule based.
> > >
> > > What definitely would change is my well tested good faith change
> waiting
> > > for review after one week could go in no matter who's on vacation or
> > > whatever. Not saying this is a problem for me today. Like I said,
> things
> > > are great around here today. Awesome.
> > >
> > >
> > > > On Aug 2, 2015, at 10:52 AM, Stack <stack@duboce.net> wrote:
> > > >
> > > > On Fri, Jul 31, 2015 at 3:15 AM, Andrew Purtell <
> > > andrew.purtell@gmail.com>
> > > > wrote:
> > > >
> > > >> I appreciate very much the earlier feedback about switching from RTC
> > to
> > > >> CTR. It helped me think about the essential thing I was after.
> > > >>
> > > >> I'm thinking of making a formal proposal to adopt this, with a VOTE:
> > > >>
> > > >>> After posting a patch to JIRA, after one week if there is no review
> > or
> > > >> veto, a committer can commit their own work.
> > > >>
> > > >> (On a bit of a lag...)
> > > >
> > > > What is the problem we are trying to solve? Blocked committers unable
> > to
> > > > get a review?
> > > >
> > > > Any commit can destabilize. Commits without peer review will
> > destabilize
> > > > more than those that have been reviewed. As a community, I'd think
> that
> > > > anything we can do to act against entropy in the system would be
> > primary
> > > > above all other considerations especially given our project a mature
> > > > datastore.
> > > >
> > > > In my own case, it seems to take me tens of patch postings to arrive
> at
> > > > something a fellow reviewer thinks worthy of commit when working
> with a
> > > > peer on an issue of substance. Often the final patch is radically
> > > different
> > > > from first posting. If auto-commit, the first version would just go
> in.
> > > > Unless review, I'd be 'done'. If review, each iteration would be in
> the
> > > > code base? Churn would go up. Commits would be more and smaller. Our
> > > > project would be made of less coherent 'change sets'.
> > > >
> > > > I'd be with LarsH suggestion that committers should be allowed go
> ahead
> > > > with one-liners or more substantial, non-controversial changes in
> site
> > or
> > > > doc but beyond this, I'd be against auto-commit.
> > > >
> > > >>
> > > >>
> > > >> Also, looking at https://hbase.apache.org/book.html#_decisions, I
> > don't
> > > >> think the "patch +1 policy" should remain because the trial OWNERS
> > > concept
> > > >> hasn't worked out, IMHO. The OWNERS concept requires a set of
> > constantly
> > > >> present and engaged owners, a resource demand that's hard to square
> > with
> > > >> the volunteer nature of our community. The amount of time any
> > committer
> > > or
> > > >> PMC member has on this project is highly variable day to day and
> week
> > to
> > > >> week.  I'm also thinking of calling a VOTE to significantly revise
> or
> > > >> strike this section.
> > > > Yeah. OWNERS project failed. Its ghost is in effect in that we each
> go
> > to
> > > > the domain expert when its clear (e.g. Matteo or Stephen on pv2).
> > > >
> > > >
> > > >> Both of these things have a common root: Volunteer time is a very
> > > precious
> > > >> commodity. Our community's supply of volunteer time fluctuates. I
> > would
> > > >> like to see committers be able to make progress with their own work
> > > even in
> > > >> periods when volunteer time is in very short supply, or when they
> are
> > > >> working on niche concerns that simply do not draw sufficient
> interest
> > > from
> > > >> other committers. (This is different from work that people think
> isn't
> > > >> appropriate - in that case ignoring it so it will go away would no
> > > longer
> > > >> be an option, a veto would be required if you want to stop
> something.)
> > > > Committers should be spending more time reviewing their peers work
> (and
> > > > just as importantly, the work of contributors)?
> > > > St.Ack
> > > >
> > > >
> > > >
> > > >> On Wed, Jul 29, 2015 at 3:56 PM, Andrew Purtell <
> > > andrew.purtell@gmail.com>
> > > >> wrote:
> > > >>
> > > >>> Had this thought after getting back on the road. As an alternative
> to
> > > any
> > > >>> sweeping change we could do one incremental but very significant
> > thing
> > > >> that
> > > >>> acknowledges our status as trusted and busy peers: After posting
a
> > > patch
> > > >> to
> > > >>> JIRA, after one week if there is no review or veto, a committer
can
> > > >> commit
> > > >>> their own work.
> > > >>>
> > > >>>
> > > >>>>> On Jul 29, 2015, at 2:20 PM, Mikhail Antonov <
> olorinbant@gmail.com
> > >
> > > >>>> wrote:
> > > >>>>
> > > >>>> Just curious, I assume if this change is made, would it only
apply
> > to
> > > >>>> master branch?
> > > >>>>
> > > >>>> -Mikhail
> > > >>>>
> > > >>>> On Wed, Jul 29, 2015 at 2:09 PM, Andrew Purtell
> > > >>>> <andrew.purtell@gmail.com> wrote:
> > > >>>>> @dev is now CCed
> > > >>>>>
> > > >>>>> I didn't want to over structure the discussion with too
much
> detail
> > > up
> > > >>> front. I do think CTR without supporting process or boundaries
can
> be
> > > >> more
> > > >>> problematic than helpful. That could take the form of customarily
> > > waiting
> > > >>> for reviews before commit even under a CTR regime. I think this
> > > >> discussion
> > > >>> has been great so far. I would also add that CTR moves 'R' from
a
> > > gating
> > > >>> requirement per commit (which can be hard to overcome for niche
> areas
> > > or
> > > >>> when volunteer resources are less available) more to RMs. will
be
> > back
> > > >>> later with more.
> > > >>>>>
> > > >>>>>
> > > >>>>>> On Jul 29, 2015, at 1:36 PM, Sean Busbey <sean.busbey@gmail.com
> >
> > > >>> wrote:
> > > >>>>>>
> > > >>>>>> I'd also favor having this discussion on dev@.
> > > >>>>>>
> > > >>>>>>> On Wed, Jul 29, 2015 at 2:29 PM, Gary Helmling
<
> > > ghelmling@gmail.com
> > > >>>
> > > >>> wrote:
> > > >>>>>>>
> > > >>>>>>> This is already a really interesting and meaningful
discussion,
> > and
> > > >> is
> > > >>>>>>> obviously important to the community.
> > > >>>>>>>
> > > >>>>>>> Is there any reason not to move this straight
to the dev@
> list?
> > > >>>>>>>
> > > >>>>>>>> On Wed, Jul 29, 2015 at 11:40 AM Todd Lipcon
<
> todd@cloudera.com
> > >
> > > >>> wrote:
> > > >>>>>>>>
> > > >>>>>>>> I'm not very active in HBase these days, so
I won't cast a
> > > non-zero
> > > >>> vote,
> > > >>>>>>>> but I'm -0 on this idea, for basically two
reasons:
> > > >>>>>>>>
> > > >>>>>>>> 1) In my experience at a past job which used
CTR, the reality
> > was
> > > >>> that it
> > > >>>>>>>> was more like "Commit and maybe review" rather
than "Commit
> then
> > > >>> review".
> > > >>>>>>>> It's always more fun (and often easier) to
write new code than
> > to
> > > >>> review
> > > >>>>>>>> other people's code, so if there isn't a requirement
that all
> > code
> > > >>> gets
> > > >>>>>>>> reviewed before commit, it's easy for the
ratio of code
> written
> > to
> > > >>> code
> > > >>>>>>>> reviewed to tend towards values significantly
greater than 1:1
> > > over
> > > >>> time.
> > > >>>>>>>> At my past job, this meant that a lot of code
made it into
> > > >>> production (it
> > > >>>>>>>> was a website) that hadn't ever been reviewed,
and in many
> cases
> > > we
> > > >>> found
> > > >>>>>>>> bugs (or other issues) that would have definitely
been caught
> > by a
> > > >>> good
> > > >>>>>>>> code reviewer.
> > > >>>>>>>>
> > > >>>>>>>> 2) CTR has both the advantage and disadvantage
of allowing
> areas
> > > of
> > > >>> code
> > > >>>>>>> to
> > > >>>>>>>> be evolved quickly by a single person. That
could be seen as a
> > > >> plus,
> > > >>> in
> > > >>>>>>>> that there are some areas which tend to get
ignored because we
> > > >> don't
> > > >>>>>>> have a
> > > >>>>>>>> critical mass of people reviewing patches
in the area --
> patches
> > > >>> languish
> > > >>>>>>>> in these areas currently. However, that could
also be seen as
> a
> > > >> good
> > > >>>>>>> thing
> > > >>>>>>>> from a "community over code" perspective --
it's better to not
> > > have
> > > >>> any
> > > >>>>>>>> areas of code with bus-factor 1. Feeling the
pain of not
> getting
> > > >>> reviews
> > > >>>>>>> in
> > > >>>>>>>> these areas with only a single active committer
encourages us
> to
> > > >> find
> > > >>>>>>>> solutions - either by deprecating "niche"
features (as we once
> > did
> > > >>> with
> > > >>>>>>>> various 'contrib' projects) or by recruiting
new committers
> who
> > > >> have
> > > >>>>>>>> interest in maintaining that code area. Lifting
review
> > > restrictions
> > > >>> would
> > > >>>>>>>> allow us to sweep bus-factor issues under
the rug.
> > > >>>>>>>>
> > > >>>>>>>> That said, I think CTR can be a valuable tool
for "move fast
> on
> > > new
> > > >>>>>>>> experimentation" type projects -- I've participated
in several
> > > >>> feature
> > > >>>>>>>> branches in HDFS where we operated on CTR
on the branch. The
> > > >>> assumption
> > > >>>>>>> was
> > > >>>>>>>> that, prior to merging the branch into trunk,
all of the
> patches
> > > >> (or
> > > >>> a
> > > >>>>>>>> consolidated mega-patch) would be thoroughly
reviewed by
> several
> > > >>> other
> > > >>>>>>>> committers. I found this to work very well
during early
> > > >> development,
> > > >>>>>>> since
> > > >>>>>>>> I could hack on things and even commit pieces
of code that had
> > > >> known
> > > >>>>>>>> issues/TODOs. For trickier patches on the
CTR branch, I still
> > > >> tended
> > > >>> to
> > > >>>>>>>> ping experienced contributors for their opinions
and feedback
> > > >> before
> > > >>>>>>>> committing.
> > > >>>>>>>>
> > > >>>>>>>> Perhaps such a hybrid policy would work well
in the context of
> > > >> HBase
> > > >>>>>>>> feature development as well?
> > > >>>>>>>>
> > > >>>>>>>> -Todd
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>> On Wed, Jul 29, 2015 at 11:27 AM, Andrew Purtell
<
> > > >>> apurtell@apache.org>
> > > >>>>>>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> Just thought of branch merge votes. Sorry
for that omission.
> I
> > > >>> think it
> > > >>>>>>>>> makes sense to keep those, but let's discuss
that too in this
> > > >>> context.
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>> On Wed, Jul 29, 2015 at 11:26 AM, Andrew
Purtell <
> > > >>> apurtell@apache.org>
> > > >>>>>>>>> wrote:
> > > >>>>>>>>>
> > > >>>>>>>>>> As Greg Stein said on a thread over
at general@incubator
> > > >>>>>>>> ("[DISCUSSION]
> > > >>>>>>>>>> Graduate Ignite from the Apache Incubator"):
> > > >>>>>>>>>>
> > > >>>>>>>>>> I always translate RTC as "we don't
trust you, so somebody
> > else
> > > >>> must
> > > >>>>>>>>>> approve anything you do." To me, that
is a lousy basis for
> > > >>> creating a
> > > >>>>>>>>>> community. Trust and peer respect
should be the basis, which
> > > >>> implies
> > > >>>>>>>> CTR.
> > > >>>>>>>>>>
> > > >>>>>>>>>> I happen to agree with this sentiment
and would like to
> > propose
> > > >>>>>>>> switching
> > > >>>>>>>>>> our consensus on commit procedure
from RTC to CTR.
> > Concurrently,
> > > >> I
> > > >>>>>>>> would
> > > >>>>>>>>>> like to propose this consensus also
include the following:
> > > >>>>>>>>>> - Checkins should pass precommit or
the committer should
> > explain
> > > >> on
> > > >>>>>>> the
> > > >>>>>>>>>> JIRA why precommit failures are in
their best judgement not
> > > >> related
> > > >>>>>>>>>> - The PMC should be willing to, ultimately,
revoke
> > committership
> > > >>>>>>> should
> > > >>>>>>>>>> trust in any individual committer
be discovered to be
> > misplaced.
> > > >> I
> > > >>>>>>>> would
> > > >>>>>>>>>> expect such a last resort to be exceedingly
rare and likely
> > > never
> > > >>> to
> > > >>>>>>>>> happen
> > > >>>>>>>>>> because of course long before that
we would be setting
> correct
> > > >>> public
> > > >>>>>>>>>> examples in the first place and respectfully
counseling well
> > > >>>>>>>> intentioned
> > > >>>>>>>>>> committers who might stray.
> > > >>>>>>>>>>
> > > >>>>>>>>>> Depending on how the conversation
proceeds here I would like
> > to
> > > >>>>>>> include
> > > >>>>>>>>>> dev@ in this conversation at the earliest
opportunity as
> > well.
> > > >>>>>>>>>>
> > > >>>>>>>>>> Thoughts?
> > > >>>>>>>>>>
> > > >>>>>>>>>> --
> > > >>>>>>>>>> Best regards,
> > > >>>>>>>>>>
> > > >>>>>>>>>> - Andy
> > > >>>>>>>>>>
> > > >>>>>>>>>> Problems worthy of attack prove their
worth by hitting
> back. -
> > > >> Piet
> > > >>>>>>>> Hein
> > > >>>>>>>>>> (via Tom White)
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>> --
> > > >>>>>>>>> Best regards,
> > > >>>>>>>>>
> > > >>>>>>>>> - Andy
> > > >>>>>>>>>
> > > >>>>>>>>> Problems worthy of attack prove their
worth by hitting back.
> -
> > > >> Piet
> > > >>>>>>> Hein
> > > >>>>>>>>> (via Tom White)
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>> --
> > > >>>>>>>> Todd Lipcon
> > > >>>>>>>> Software Engineer, Cloudera
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> --
> > > >>>>>> Sean
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>> --
> > > >>>> Thanks,
> > > >>>> Michael Antonov
> > > >>
> > >
> >
> >
> >
> > --
> > Sean
> >
>
>
>
> --
> Best regards,
>
>    - Andy
>
> Problems worthy of attack prove their worth by hitting back. - Piet Hein
> (via Tom White)
>

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