zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Han <h...@cloudera.com>
Subject Re: reconfig APIs
Date Thu, 05 Jan 2017 20:17:55 GMT
Patch looking good for ZOOKEEPER-2642 (
https://github.com/apache/zookeeper/pull/122).

I just tested locally with both stress unit tests and zkCli.sh, everything
looks fine. If anyone has any cycles could you please review / test this
patch? I'd like to have more +1s before getting this in - and it would be
good if we can get this in sooner so we can cut out a release candidate
build for 3.5.3 (beta).

On Thu, Dec 8, 2016 at 2:50 PM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:

> Thanks for the replies everyone. Adding the methods back with a
> deprecation notice solves Curator’s problems so I’ll proceed with that on
> ZOOKEEPER-2642.
>
> -Jordan
>
> > On Dec 8, 2016, at 6:56 PM, Michael Han <hanm@cloudera.com> wrote:
> >
> > Thanks Jordan for raising the concern. It's a reasonable concern with
> good
> > intention to make user's life easier, which is important to the project.
> >
> > There are two separate issues we need to reach a consensus here:
> >
> > * Move reconfig API
> > The details of why the decision was made is documented in ZOOKEEPER-2014
> > JIRA, but just to recap, the API is moved because it's the right thing to
> > do in long term, which yield a cleaner interface design for ZooKeeper
> > client API. In future, we might also want to clean up other APIs (i.e.
> move
> > quota related APIs over as well.) as well. While seemingly stylish as it
> > might look like, I believe a clean API is also important for a project's
> > usability / adoption. From the discussions so far I think everyone either
> > agree with this, or have no strong opinion against it, so I consider this
> > question is solved.
> >
> > * Backward compatibility between releases
> > Since we decide refactor reconfig API, the question is when. The reconfig
> > API was removed from ZOOKEEPER-2014 (which targets 3.5.3 alpha) because
> of
> > the definition of alpha / beta releases in the context of ZooKeeper
> project
> > (I'm not saying this is right or wrong, simply state a fact up to date):
> an
> > alpha release implies that we allow backward incompatible changes, and a
> > beta release implies that the APIs are locked down and none backward
> > compatible change. Given this definition, it is perfectly legitimate to
> > move API around w/o considering compatibility. Now what Jordan pointed
> out
> > regarding definition of 'alpha' combined with some old discussion
> threads I
> > found in user list indicate that our definition of 'alpha', and 'beta' is
> > none standard and could cause confusions. So I think we should try reach
> a
> > consensus on whether we stick with current definition of 'alpha' and
> 'beta'
> > release during 3.5.3 release cycle, or adopt something else.
> >
> > With all these said, I think options are:
> > * Do nothing, stick with current alpha / beta definition.
> > * Try redefine what 'alpha' and 'beta' means in the context of ZooKeeper
> > and then go from there in 3.5.3 release cycle.
> >
> > In any cases, I'd like to propose we keep the reconfig API in
> > ZooKeeperAdmin which as previously stated is the right thing to do in
> long
> > term. If we decide to maintain backward API compatibility for 3.5.3-beta
> > release, we could add reconfig API back to ZooKeeper but mark it as
> > deprecated, then remove the API in next major release (3.6 or 4.x).
> >
> >
> > On Thu, Dec 8, 2016 at 8:27 AM, Patrick Hunt <phunt@apache.org> wrote:
> >
> >> I've only seen a few questions about versioning come up on the ZK lists,
> >> they were quickly responded to with pointers to the process. iirc we
> based
> >> our versioning scheme on what Hadoop was/is using. Additionally if we
> don't
> >> allow for alpha time it will further slow progress as there will be no
> >> opportunity to adjust things like APIs once they are committed and
> >> generally available for people to "kick the tires".
> >>
> >> I'll leave it up to the rest of the community to state their opinions,
> but
> >> as i have stated imo it would be a mistake to revert this change. Jordan
> >> has raised a reasonable concern - I consider this a blocker for
> 3.5.3-alpha
> >> until it is resolved.
> >>
> >> Patrick
> >>
> >> On Thu, Dec 8, 2016 at 1:46 AM, Jordan Zimmerman <
> >> jordan@jordanzimmerman.com
> >>> wrote:
> >>
> >>>> I think people understand what alpha means.
> >>>
> >>> With respect, the ZooKeeper team has used “alpha” in a novel way that
> is
> >>> sowing considerable confusion. I wrote emails about this a while back.
> >> But,
> >>> here again, is another case where the non-standard usage of “alpha”
> will
> >>> cause confusion. To reiterate: someone who sees "3.5.2-alpha” will
> think
> >>> that there will eventually be a “3.5.2-beta” and finally a “3.5.2”
> >> release
> >>> version. But, with ZooKeeper there will never be a “3.5.2” release.
In
> >>> fact, the “-alpha” label is mis-located. The real version is
> >>> “3.5.?-alpha1”, “3.5.?-alpha2”, etc. There’s an important implication
> >> here.
> >>> If a ZooKeeper user who doesn’t follow the mailing lists, etc. sees
> >>> “3.5.2-alpha” and “3.5.3-alpha” they will mentally see these as
two
> >>> different releases. What ZOOKEEPER-2014 has done is remove an existing
> >> API
> >>> from what appears to be a released version 3.5.2 (which never really
> >>> existed). This change violates semantic versioning. For external users,
> >> the
> >>> version after “3.5.2” should be “4.x.x” as it has breaking changes.
> >>>
> >>>> It's not about style, there were a number of concerns addressed in
> that
> >>>> patch.
> >>>
> >>> The auth issues are very real ones. The issues in ZOOKEEPER-2014 can be
> >>> addressed completely without moving the reconfig() methods to a new
> >> class.
> >>> It may be useful to move APIs around for clarity, etc. but these
> breaking
> >>> changes should be for a version that signals breaking changes - 4.x.x
> or
> >>> something. i.e. moving the reconfig() APIs is orthogonal to the
> concerns
> >> of
> >>> ZOOKEEPER-2014 and should be a separate Jira issue.
> >>>
> >>>> I think people understand what alpha means. There may be some short
> >> term
> >>>> impact for a few, but a significant benefit over the long term.
> >>>
> >>> Again with respect - 3.5.0-alpha was made available in Maven Central
> >>> August 2014 - over 2 years ago. Regardless of the ZooKeeper team’s
> >> intent,
> >>> this is NOT an alpha in users’ minds. This is a released library that
> is
> >> in
> >>> production in major organizations. The ZooKeeper team should realize
> this
> >>> and adjust their thinking about the “alpha” tag. For Apache Curator,
> >> we’re
> >>> now put in a position where the reconfig() APIs have disappeared. In
> >> order
> >>> to maintain our own internal semantic versioning we will have to
> >> consider a
> >>> third branch of Curator (we currently have a ZooKeeper 3.4.x compatible
> >>> version and a ZooKeeper 3.5.x compatible version). It would be awesome
> if
> >>> we didn’t have to do this.
> >>>
> >>> -Jordan
> >>>
> >>>> On Dec 7, 2016, at 7:16 PM, Patrick Hunt <phunt@apache.org> wrote:
> >>>>
> >>>> It's not about style, there were a number of concerns addressed in
> that
> >>>> patch. We didn't take the change lightly, we've been discussing it
> over
> >>>> jira and the mailing list over the past two years.
> >>>>
> >>>> I think people understand what alpha means. There may be some short
> >> term
> >>>> impact for a few, but a significant benefit over the long term.
> >>>>
> >>>> Patrick
> >>>>
> >>>> On Dec 7, 2016 9:24 AM, "Jordan Zimmerman" <
> jordan@jordanzimmerman.com
> >>>
> >>>> wrote:
> >>>>
> >>>>> I read through the issue and disagree about the decision to move
the
> >>> APIs
> >>>>> out. That was a stylistic choice that breaks client code. I realize
> >> that
> >>>>> 3.5.x has been advertised as an alpha but you must realize that
most
> >> of
> >>> the
> >>>>> world is using it in production. These APIs have now been published.
> >>> This
> >>>>> will create a real headache for Curator which is why I’ve created
> >>>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2642 <
> >>>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2642> - I hope
we
> can
> >>>>> move these APIs back into ZooKeeper.java.
> >>>>>
> >>>>> -Jordan
> >>>>>
> >>>>>> On Dec 7, 2016, at 5:54 PM, Patrick Hunt <phunt@apache.org>
wrote:
> >>>>>>
> >>>>>> It's discussed in more depth in the JIRA iirc, but basically;
> >>>>>> ZooKeeper.java provides client APIs, reconfig is an admiistrative
> >> API.
> >>>>>>
> >>>>>> Patrick
> >>>>>>
> >>>>>> On Wed, Dec 7, 2016 at 8:48 AM, Jordan Zimmerman <
> >>>>> jordan@jordanzimmerman.com
> >>>>>>> wrote:
> >>>>>>
> >>>>>>> I understand the need to make the methods require proper
auth but
> >>>>> there's
> >>>>>>> no reason to move it to a different class that I can see.
Am I
> >> missing
> >>>>>>> something?
> >>>>>>>
> >>>>>>> ====================
> >>>>>>> Jordan Zimmerman
> >>>>>>>
> >>>>>>>> On Dec 7, 2016, at 4:37 PM, Patrick Hunt <phunt@apache.org>
> wrote:
> >>>>>>>>
> >>>>>>>> This problem has been a long standing blocker issue
for 3.5 and
> >>>>>>> identified
> >>>>>>>> early on as something that would need to change. This
has been one
> >> of
> >>>>> the
> >>>>>>>> reasons why 3.5 has stayed in alpha - because we allow
> non-backward
> >>>>>>>> compatible changes to new APIs in alpha and we knew
we would have
> >> to
> >>>>> fix
> >>>>>>>> this. The description/comments of ZOOKEEPER-2014 does
a good job
> >>>>>>>> documenting why this had to change.
> >>>>>>>>
> >>>>>>>> Patrick
> >>>>>>>>
> >>>>>>>> On Wed, Dec 7, 2016 at 3:18 AM, Jordan Zimmerman <
> >>>>>>> jordan@jordanzimmerman.com
> >>>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> OK - I found the offending issue: ZOOKEEPER-2014
> >>>>>>>>>
> >>>>>>>>> What is the benefit/logic of moving the reconfig()
variants into
> a
> >>> new
> >>>>>>>>> class? I can see if this was done from the start
but you have now
> >>>>> broken
> >>>>>>>>> Curator in a fairly serious non-backward compatible
way for a
> >> minor
> >>>>>>>>> documenting benefit. Does anyone object to me reversing
this?
> >>>>>>>>>
> >>>>>>>>> -Jordan
> >>>>>>>>>
> >>>>>>>>>> On Dec 7, 2016, at 11:37 AM, Jordan Zimmerman
<
> >>>>>>>>> jordan@jordanzimmerman.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> I was compiling Curator against the ZK master
and noticed that
> >> the
> >>>>>>>>> reconfig APIs are gone/changed. Can anyone point
me at the issues
> >>> for
> >>>>>>> this
> >>>>>>>>> and/or the discussion why this breaking change was
made?
> >>>>>>>>>>
> >>>>>>>>>> -Jordan
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>
> >
> >
> >
> > --
> > Cheers
> > Michael.
>
>


-- 
Cheers
Michael.

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