You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@zookeeper.apache.org by Chris Darroch <ch...@pearsoncmg.com> on 2009/02/04 20:27:07 UTC

ZooKeeper 3.1 and C API/ABI

Hi --

   I notice that 3.1.0 is on its way and it includes ZOOKEEPER-255 which
adds the Stat structure as a parameter to the zoo_set() C call.  This is
a valuable change and I don't want to hold it up.

   However, I thought I should point out that this kind of change
breaks the API and ABI.  For major Apache C projects like the APR,
such breakage is allowed only with a major version number change:

http://apr.apache.org/versioning.html#source

   Following such guidelines, I suppose, the old zoo_*set() functions
would remain as-is until 4.0.0, and parallel zoo_*set2() or
zoo_stat_*set() functions would add the new functionality.


    Now, fair do's, ZooKeeper may not care as much as APR or httpd,
since it's mostly a Java project.  At a minimum, though, it would be
excellent if there was compile-time versioning information available
so that external projects could check and, at a bare minimum, fail to
compile if the API/ABI has changed.  APR has some useful guidelines
making compile-time constants (e.g., ZOO_MAJOR_VERSION) available:

http://apr.apache.org/versioning.html#vsncheck

   Speaking personally, one really nice aspect of working with APR
for me is the parallel installation framework.  Again, this might be
overkill for ZK, but I'll just point it out as well:

http://apr.apache.org/versioning.html#parallel

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B

Re: Testing Zookeeper

Posted by Patrick Hunt <ph...@apache.org>.
Yes, please create a new JIRA!

I'd also encourage you to subscribe to the zookeeper-dev list (please 
followup/discussion there). All of our technical discussion goes on 
there - zookeeper-user is more for support issues on released products.

Thanks,

Patrick

Joshua Tuberville wrote:
> Nitay,
> 
> Thanks for pointing out your ticket.  I assumed someone had already done the same.  I will take a look at your patch and compare to our code.  I agree that there should be some common way for tests both internal and external to buildup a server and tear it down.
> 
> Joshua
> 
> -----Original Message-----
> From: Nitay [mailto:nitayj@gmail.com] 
> Sent: Tuesday, February 10, 2009 12:46 PM
> To: zookeeper-user@hadoop.apache.org
> Subject: Re: Testing Zookeeper
> 
> Joshua,
> 
> There may already be some JIRAs open regarding this, e.g.
> https://issues.apache.org/jira/browse/ZOOKEEPER-278. You can assign those to
> yourself and attach your stuff there if it fits your issue.
> 
> On Tue, Feb 10, 2009 at 11:44 AM, Mahadev Konar <ma...@yahoo-inc.com>wrote:
> 
>> HI Joshua,
>>  Feel free to open a jira and attach a patch.
>>
>> Please take a look at how to contribute:
>>
>> http://wiki.apache.org/hadoop/ZooKeeper/HowToContribute
>>
>> Thanks
>> mahadev
>>
>> On 2/10/09 11:34 AM, "Joshua Tuberville" <Jo...@eharmony.com>
>> wrote:
>>
>>> To test our zookeeper usage we built a utility class using some of the
>> methods
>>> in org.apache.zookeeper.test.ClientBase out of the test folder.  This
>> allows
>>> testing to be done using any framework JUnit4, JUnit5, TestNG, etc.  We
>> would
>>> prefer this be in the zookeeper jar.  Should I open a JIRA item and
>> include
>>> the class?
>>>
>>> Thanks,
>>> Joshua
>>

Re: Testing Zookeeper

Posted by Mahadev Konar <ma...@yahoo-inc.com>.
Hi Nitay and Joshua,
  It would be great if in the future we could keep these discussions about
development and aptches on zookeeper-dev rather than the user list. The user
list is supposed to be used for released versions and questions from users.

Thanks
mahadev


On 2/10/09 12:51 PM, "Joshua Tuberville" <Jo...@eharmony.com>
wrote:

> Nitay,
> 
> Thanks for pointing out your ticket.  I assumed someone had already done the
> same.  I will take a look at your patch and compare to our code.  I agree that
> there should be some common way for tests both internal and external to
> buildup a server and tear it down.
> 
> Joshua
> 
> -----Original Message-----
> From: Nitay [mailto:nitayj@gmail.com]
> Sent: Tuesday, February 10, 2009 12:46 PM
> To: zookeeper-user@hadoop.apache.org
> Subject: Re: Testing Zookeeper
> 
> Joshua,
> 
> There may already be some JIRAs open regarding this, e.g.
> https://issues.apache.org/jira/browse/ZOOKEEPER-278. You can assign those to
> yourself and attach your stuff there if it fits your issue.
> 
> On Tue, Feb 10, 2009 at 11:44 AM, Mahadev Konar <ma...@yahoo-inc.com>wrote:
> 
>> HI Joshua,
>>  Feel free to open a jira and attach a patch.
>> 
>> Please take a look at how to contribute:
>> 
>> http://wiki.apache.org/hadoop/ZooKeeper/HowToContribute
>> 
>> Thanks
>> mahadev
>> 
>> On 2/10/09 11:34 AM, "Joshua Tuberville" <Jo...@eharmony.com>
>> wrote:
>> 
>>> To test our zookeeper usage we built a utility class using some of the
>> methods
>>> in org.apache.zookeeper.test.ClientBase out of the test folder.  This
>> allows
>>> testing to be done using any framework JUnit4, JUnit5, TestNG, etc.  We
>> would
>>> prefer this be in the zookeeper jar.  Should I open a JIRA item and
>> include
>>> the class?
>>> 
>>> Thanks,
>>> Joshua
>> 
>> 


RE: Testing Zookeeper

Posted by Joshua Tuberville <Jo...@eharmony.com>.
Nitay,

Thanks for pointing out your ticket.  I assumed someone had already done the same.  I will take a look at your patch and compare to our code.  I agree that there should be some common way for tests both internal and external to buildup a server and tear it down.

Joshua

-----Original Message-----
From: Nitay [mailto:nitayj@gmail.com] 
Sent: Tuesday, February 10, 2009 12:46 PM
To: zookeeper-user@hadoop.apache.org
Subject: Re: Testing Zookeeper

Joshua,

There may already be some JIRAs open regarding this, e.g.
https://issues.apache.org/jira/browse/ZOOKEEPER-278. You can assign those to
yourself and attach your stuff there if it fits your issue.

On Tue, Feb 10, 2009 at 11:44 AM, Mahadev Konar <ma...@yahoo-inc.com>wrote:

> HI Joshua,
>  Feel free to open a jira and attach a patch.
>
> Please take a look at how to contribute:
>
> http://wiki.apache.org/hadoop/ZooKeeper/HowToContribute
>
> Thanks
> mahadev
>
> On 2/10/09 11:34 AM, "Joshua Tuberville" <Jo...@eharmony.com>
> wrote:
>
> > To test our zookeeper usage we built a utility class using some of the
> methods
> > in org.apache.zookeeper.test.ClientBase out of the test folder.  This
> allows
> > testing to be done using any framework JUnit4, JUnit5, TestNG, etc.  We
> would
> > prefer this be in the zookeeper jar.  Should I open a JIRA item and
> include
> > the class?
> >
> > Thanks,
> > Joshua
>
>

Re: Testing Zookeeper

Posted by Nitay <ni...@gmail.com>.
Joshua,

There may already be some JIRAs open regarding this, e.g.
https://issues.apache.org/jira/browse/ZOOKEEPER-278. You can assign those to
yourself and attach your stuff there if it fits your issue.

On Tue, Feb 10, 2009 at 11:44 AM, Mahadev Konar <ma...@yahoo-inc.com>wrote:

> HI Joshua,
>  Feel free to open a jira and attach a patch.
>
> Please take a look at how to contribute:
>
> http://wiki.apache.org/hadoop/ZooKeeper/HowToContribute
>
> Thanks
> mahadev
>
> On 2/10/09 11:34 AM, "Joshua Tuberville" <Jo...@eharmony.com>
> wrote:
>
> > To test our zookeeper usage we built a utility class using some of the
> methods
> > in org.apache.zookeeper.test.ClientBase out of the test folder.  This
> allows
> > testing to be done using any framework JUnit4, JUnit5, TestNG, etc.  We
> would
> > prefer this be in the zookeeper jar.  Should I open a JIRA item and
> include
> > the class?
> >
> > Thanks,
> > Joshua
>
>

Re: Testing Zookeeper

Posted by Mahadev Konar <ma...@yahoo-inc.com>.
HI Joshua,
  Feel free to open a jira and attach a patch.

Please take a look at how to contribute:

http://wiki.apache.org/hadoop/ZooKeeper/HowToContribute

Thanks
mahadev

On 2/10/09 11:34 AM, "Joshua Tuberville" <Jo...@eharmony.com>
wrote:

> To test our zookeeper usage we built a utility class using some of the methods
> in org.apache.zookeeper.test.ClientBase out of the test folder.  This allows
> testing to be done using any framework JUnit4, JUnit5, TestNG, etc.  We would
> prefer this be in the zookeeper jar.  Should I open a JIRA item and include
> the class?
> 
> Thanks,
> Joshua


Testing Zookeeper

Posted by Joshua Tuberville <Jo...@eharmony.com>.
To test our zookeeper usage we built a utility class using some of the methods in org.apache.zookeeper.test.ClientBase out of the test folder.  This allows testing to be done using any framework JUnit4, JUnit5, TestNG, etc.  We would prefer this be in the zookeeper jar.  Should I open a JIRA item and include the class?

Thanks,
Joshua

Re: ZooKeeper 3.1 and C API/ABI

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Patrick Hunt wrote:

> Chris, please take a look at the patch on 293 asap and let us know if 
> you have any issues -- I will be spinning a new release once mahadev/ben 
> review and commit the change.

   That looks great -- covers a bunch of things, thanks!

> ps. I noticed you had some additional suggestions for the c code in 
> JIRA, thanks. FYI we do accept contributions from anyone. ;-)

   Yes, I've submitted a couple of patches (see ZOOKEEPER-262) and
written an httpd module for ZooKeeper, and hope to write another one
when the slotmem API is baked.  The small-object cache one is available
from my Apache page:

http://people.apache.org/~chrisd/projects/shared_map/

   But it's all a question of spare time.  :-)  I hope to have another
more generally useful package available shortly; I'll put it up on
my Apache page when its ready for general testing and use.  Working
on that has been what turned up some of the issues I submitted
(e.g., deallocating memory per ZOOKEEPER-294).

   Cheers,

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: ZooKeeper 3.1 and C API/ABI

Posted by Patrick Hunt <ph...@apache.org>.
Chris, please take a look at the patch on 293 asap and let us know if 
you have any issues -- I will be spinning a new release once mahadev/ben 
review and commit the change.

Patrick

ps. I noticed you had some additional suggestions for the c code in 
JIRA, thanks. FYI we do accept contributions from anyone. ;-)


Patrick Hunt wrote:
> Chris, that's unfortunate re the version number (config.h), but I think 
> I see why that is -- config.h should only really be visible in the 
> implementation, not exposed through the includes.
> 
> I've created a JIRA for this:
> https://issues.apache.org/jira/browse/ZOOKEEPER-293
> 
> We'll hold 3.1 for this JIRA, I'll create a new release candidate when 
> the patch is ready. (hopefully today)
> 
> Ben, Mahadev please be available to review/fasttrack this JIRA.
> 
> Patrick
> 
> Chris Darroch wrote:
>> Hi --
>>
>>>> Btw, the version is in the config.h file, generated by autotools, as 
>>>> VERSION. We don't break that out as individual parameters but we can 
>>>> if there is interest.
>>>
>>>    That's useful, I'd missed that.  Thanks; that should work for me
>>> for now.
>>
>>   Sorry ... on second glance, I'll have to retract that.  The problem
>> here is that config.h doesn't get installed by "make install", it's
>> just used by the autoconf stuff.  So there's no simple way that I'm
>> aware of to check the C API version at compile time.
>>
>>   For 3.1.0, I'd suggest either reverting ZOOKEEPER-255 until 4.0.0,
>> or making sure there's at least a way of determining the API version
>> using C macros.  For example, I'd want to be able to do something like:
>>
>> #if ZOO_MAJOR_VERSION >= 3 && ZOO_MINOR_VERSION >= 1
>>    zoo_set(..., stat);
>> #else
>>    zoo_set(...);
>> #endif
>>
>>   Ideally, as I mentioned, until 4.0.0 the zoo_set() functionality
>> would be moved to a zoo_stat_set() or zoo_set_exec() function,
>> and zoo_set() would keep its existing definition but become just
>> a wrapper that invoked the new function with a NULL stat argument.
>> That would be the "APR way", I think, of handling this situation.
>> With the next major version the new function with the extra argument
>> could be renamed back to zoo_set().
>>
>>   It's slightly ugly, I know, if you're thinking of this as a bug
>> which needs to be fixed urgently.  If you're not concerned about
>> backward API compatibility, at a minimum I'd request externally
>> visible macros in zookeeper.h for 3.1.0:
>>
>> #define ZOO_MAJOR_VERSION 3
>> #define ZOO_MINOR_VERSION 1
>> #define ZOO_PATCH_VERSION 0
>>
>>   Thanks,
>>
>> Chris.
>>

Re: ZooKeeper 3.1 and C API/ABI

Posted by Patrick Hunt <ph...@apache.org>.
Chris, that's unfortunate re the version number (config.h), but I think 
I see why that is -- config.h should only really be visible in the 
implementation, not exposed through the includes.

I've created a JIRA for this:
https://issues.apache.org/jira/browse/ZOOKEEPER-293

We'll hold 3.1 for this JIRA, I'll create a new release candidate when 
the patch is ready. (hopefully today)

Ben, Mahadev please be available to review/fasttrack this JIRA.

Patrick

Chris Darroch wrote:
> Hi --
> 
>>> Btw, the version is in the config.h file, generated by autotools, as 
>>> VERSION. We don't break that out as individual parameters but we can 
>>> if there is interest.
>>
>>    That's useful, I'd missed that.  Thanks; that should work for me
>> for now.
> 
>   Sorry ... on second glance, I'll have to retract that.  The problem
> here is that config.h doesn't get installed by "make install", it's
> just used by the autoconf stuff.  So there's no simple way that I'm
> aware of to check the C API version at compile time.
> 
>   For 3.1.0, I'd suggest either reverting ZOOKEEPER-255 until 4.0.0,
> or making sure there's at least a way of determining the API version
> using C macros.  For example, I'd want to be able to do something like:
> 
> #if ZOO_MAJOR_VERSION >= 3 && ZOO_MINOR_VERSION >= 1
>    zoo_set(..., stat);
> #else
>    zoo_set(...);
> #endif
> 
>   Ideally, as I mentioned, until 4.0.0 the zoo_set() functionality
> would be moved to a zoo_stat_set() or zoo_set_exec() function,
> and zoo_set() would keep its existing definition but become just
> a wrapper that invoked the new function with a NULL stat argument.
> That would be the "APR way", I think, of handling this situation.
> With the next major version the new function with the extra argument
> could be renamed back to zoo_set().
> 
>   It's slightly ugly, I know, if you're thinking of this as a bug
> which needs to be fixed urgently.  If you're not concerned about
> backward API compatibility, at a minimum I'd request externally
> visible macros in zookeeper.h for 3.1.0:
> 
> #define ZOO_MAJOR_VERSION 3
> #define ZOO_MINOR_VERSION 1
> #define ZOO_PATCH_VERSION 0
> 
>   Thanks,
> 
> Chris.
> 

Re: ZooKeeper 3.1 and C API/ABI

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Hi --

>> Btw, the version is in the config.h file, generated by autotools, as 
>> VERSION. We don't break that out as individual parameters but we can if 
>> there is interest.
> 
>    That's useful, I'd missed that.  Thanks; that should work for me
> for now.

   Sorry ... on second glance, I'll have to retract that.  The problem
here is that config.h doesn't get installed by "make install", it's
just used by the autoconf stuff.  So there's no simple way that I'm
aware of to check the C API version at compile time.

   For 3.1.0, I'd suggest either reverting ZOOKEEPER-255 until 4.0.0,
or making sure there's at least a way of determining the API version
using C macros.  For example, I'd want to be able to do something like:

#if ZOO_MAJOR_VERSION >= 3 && ZOO_MINOR_VERSION >= 1
    zoo_set(..., stat);
#else
    zoo_set(...);
#endif

   Ideally, as I mentioned, until 4.0.0 the zoo_set() functionality
would be moved to a zoo_stat_set() or zoo_set_exec() function,
and zoo_set() would keep its existing definition but become just
a wrapper that invoked the new function with a NULL stat argument.
That would be the "APR way", I think, of handling this situation.
With the next major version the new function with the extra argument
could be renamed back to zoo_set().

   It's slightly ugly, I know, if you're thinking of this as a bug
which needs to be fixed urgently.  If you're not concerned about
backward API compatibility, at a minimum I'd request externally
visible macros in zookeeper.h for 3.1.0:

#define ZOO_MAJOR_VERSION 3
#define ZOO_MINOR_VERSION 1
#define ZOO_PATCH_VERSION 0

   Thanks,

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B

Re: ZooKeeper 3.1 and C API/ABI

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Benjamin Reed wrote:

> you are correct we usually increment the version number on an API breakage.
> in the olden days if you called a function with less parameters than
> expected, a null would get passed. if this still happens we are ABI
> compatible. (i haven't tried it though...)

   Yeah, I wondered about that; it's not something I'd want to assume
worked on every platform, I think.


Patrick Hunt wrote:

> Btw, the version is in the config.h file, generated by autotools, as 
> VERSION. We don't break that out as individual parameters but we can if 
> there is interest.

   That's useful, I'd missed that.  Thanks; that should work for me
for now.

> To get this worked out and working will will take some time. If it's ok 
> with you, and if there is community interest in doing this, why don't we 
> address these process changes in 3.2? Please enter a JIRA to document 
> this for 3.2 and we'll work something out. We'll also do a better job of 
> documenting exactly what the rules are related to non-bw compat api 
> changes and version numbering.

   I'll think about doing that, sure.  FWIW, there are also the libtool
conventions for library file naming to consider.

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


RE: ZooKeeper 3.1 and C API/ABI

Posted by Benjamin Reed <br...@yahoo-inc.com>.
you are correct we usually increment the version number on an API breakage. in the olden days if you called a function with less parameters than expected, a null would get passed. if this still happens we are ABI compatible. (i haven't tried it though...)

ben

________________________________________
From: Chris Darroch [chrisd@pearsoncmg.com]
Sent: Wednesday, February 04, 2009 11:27 AM
To: zookeeper-user@hadoop.apache.org
Subject: ZooKeeper 3.1 and C API/ABI

Hi --

   I notice that 3.1.0 is on its way and it includes ZOOKEEPER-255 which
adds the Stat structure as a parameter to the zoo_set() C call.  This is
a valuable change and I don't want to hold it up.

   However, I thought I should point out that this kind of change
breaks the API and ABI.  For major Apache C projects like the APR,
such breakage is allowed only with a major version number change:

http://apr.apache.org/versioning.html#source

   Following such guidelines, I suppose, the old zoo_*set() functions
would remain as-is until 4.0.0, and parallel zoo_*set2() or
zoo_stat_*set() functions would add the new functionality.


    Now, fair do's, ZooKeeper may not care as much as APR or httpd,
since it's mostly a Java project.  At a minimum, though, it would be
excellent if there was compile-time versioning information available
so that external projects could check and, at a bare minimum, fail to
compile if the API/ABI has changed.  APR has some useful guidelines
making compile-time constants (e.g., ZOO_MAJOR_VERSION) available:

http://apr.apache.org/versioning.html#vsncheck

   Speaking personally, one really nice aspect of working with APR
for me is the parallel installation framework.  Again, this might be
overkill for ZK, but I'll just point it out as well:

http://apr.apache.org/versioning.html#parallel

Chris.

--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B

Re: ZooKeeper 3.1 and C API/ABI

Posted by Patrick Hunt <ph...@apache.org>.
Hi Chris, these are good points, thanks for the feedback.

For java we have the ability to deprecate so we do as you mention - 
major version if the API changes in non-bw compatible way. However we 
haven't been enforcing that constraint for the C interface, perhaps we 
should. BTW, we do list 255 in the release notes (part of the release 
and also on the doc pages) as an incompatibility from 3.0.1.

I think part of the issue in 255 was that we considered this a bug more 
than anything (not a feature or improvement) -- but the ABI issue you 
mention still stands even if that's the case.

Btw, the version is in the config.h file, generated by autotools, as 
VERSION. We don't break that out as individual parameters but we can if 
there is interest.

To get this worked out and working will will take some time. If it's ok 
with you, and if there is community interest in doing this, why don't we 
address these process changes in 3.2? Please enter a JIRA to document 
this for 3.2 and we'll work something out. We'll also do a better job of 
documenting exactly what the rules are related to non-bw compat api 
changes and version numbering.

Patrick

Chris Darroch wrote:
> Hi --
> 
>   I notice that 3.1.0 is on its way and it includes ZOOKEEPER-255 which
> adds the Stat structure as a parameter to the zoo_set() C call.  This is
> a valuable change and I don't want to hold it up.
> 
>   However, I thought I should point out that this kind of change
> breaks the API and ABI.  For major Apache C projects like the APR,
> such breakage is allowed only with a major version number change:
> 
> http://apr.apache.org/versioning.html#source
> 
>   Following such guidelines, I suppose, the old zoo_*set() functions
> would remain as-is until 4.0.0, and parallel zoo_*set2() or
> zoo_stat_*set() functions would add the new functionality.
> 
> 
>    Now, fair do's, ZooKeeper may not care as much as APR or httpd,
> since it's mostly a Java project.  At a minimum, though, it would be
> excellent if there was compile-time versioning information available
> so that external projects could check and, at a bare minimum, fail to
> compile if the API/ABI has changed.  APR has some useful guidelines
> making compile-time constants (e.g., ZOO_MAJOR_VERSION) available:
> 
> http://apr.apache.org/versioning.html#vsncheck
> 
>   Speaking personally, one really nice aspect of working with APR
> for me is the parallel installation framework.  Again, this might be
> overkill for ZK, but I'll just point it out as well:
> 
> http://apr.apache.org/versioning.html#parallel
> 
> Chris.
>