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 Sat, 11 Feb 2017 23:43:40 GMT
Both pull requests are now merged in master and branch-3.5. Please let me
know if you spot any issues.

I've also reopened ZOOKEEPER-2635
<https://issues.apache.org/jira/browse/ZOOKEEPER-2635> as a reminder to
regenerate documents for 3.5.3 release.

On Sat, Feb 4, 2017 at 10:43 PM, Michael Han <hanm@cloudera.com> wrote:

> We now have two pull requests pending for this issue per discussion in
> https://github.com/apache/zookeeper/pull/122
>
> The PR for master branch is now: #152
> <https://github.com/apache/zookeeper/pull/152>
> The PR for branch-3.5 is now: #151
> <https://github.com/apache/zookeeper/pull/151>
>
> I've tested both pull requests locally and everything looks good. Please
> have a look if anyone has any cycles next week - I plan to merge both by
> end of next week (2/10) so we can get 3.5.3 release moving forward.
>
> On Thu, Jan 5, 2017 at 12:17 PM, Michael Han <hanm@cloudera.com> wrote:
>
>> Patch looking good for ZOOKEEPER-2642 (https://github.com/apache/zoo
>> keeper/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.
>>
>
>
>
> --
> Cheers
> Michael.
>



-- 
Cheers
Michael.

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