You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Patrick Hunt <ph...@apache.org> on 2009/10/01 00:10:39 UTC

Re: feedback zkclient

Hi Stefan, two suggestions off the bat:

1) fill in something in the README, doesn't have to be final or 
polished, but give some insight into the what/why/how/where/goals/etc... 
to get things moving quickly for reviewers & new users.

2) you should really discuss on the dev list. It's up to you to include 
user, but apache discourages use of user for development discussion 
(plus you'll pickup more developer insight there)

Patrick

Stefan Groschupf wrote:
> Hi Zookeeper developer,
> it would be great if you guys could give us some feedback about our 
> project zkclient.
> http://github.com/joa23/zkclient
> The main idea is making the life of lazy developers that only want 
> minimal zk functionality much easier.
> 
> We have a functionality like zkclient mock making testing easy and fast 
> without running a real zkserver, simple call back interfaces for the 
> different event types, reconnecting handling in case of timeout etc.
> 
> We feel we come closer to a release so it would be great if some experts 
> could have a look and give us some feedback.
> Thanks,
> Stefan
> 
> 
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Hadoop training and consulting
> http://www.scaleunlimited.com
> http://www.101tec.com
> 
> 
> 

Re: feedback zkclient

Posted by Stefan Groschupf <sg...@101tec.com>.
Hi Patrick,
thanks - we will do, also thanks for all the feedback.
Stefan

On Oct 5, 2009, at 11:22 AM, Patrick Hunt wrote:

> You might want to add a link to zkclient on this page:
>
> http://wiki.apache.org/hadoop/ZooKeeper/UsefulTools
>
> Patrick
>
> Patrick Hunt wrote:
>> Ted Dunning wrote:
>>> Judging by history and that fact that only 40/127 issues are  
>>> resolved, 3.3
>>> is probably 3-6 months away.  Is that a fair assessment?
>> Yes, that's fair.
>> Patrick
>>> On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org>  
>>> wrote:
>>>
>>>> One nice thing about ephemeral is that the Stat contains the owner
>>>> sessionid. As you say, it's highly implementation dependent. It's  
>>>> also
>>>> something we recognize is a problem for users, we've slated it  
>>>> for 3.3.0
>>>> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>>>>
>>>
>>>
>>>
>

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Hadoop training and consulting
http://www.scaleunlimited.com
http://www.101tec.com




Re: feedback zkclient

Posted by Stefan Groschupf <sg...@101tec.com>.
Hi Patrick,
thanks - we will do, also thanks for all the feedback.
Stefan

On Oct 5, 2009, at 11:22 AM, Patrick Hunt wrote:

> You might want to add a link to zkclient on this page:
>
> http://wiki.apache.org/hadoop/ZooKeeper/UsefulTools
>
> Patrick
>
> Patrick Hunt wrote:
>> Ted Dunning wrote:
>>> Judging by history and that fact that only 40/127 issues are  
>>> resolved, 3.3
>>> is probably 3-6 months away.  Is that a fair assessment?
>> Yes, that's fair.
>> Patrick
>>> On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org>  
>>> wrote:
>>>
>>>> One nice thing about ephemeral is that the Stat contains the owner
>>>> sessionid. As you say, it's highly implementation dependent. It's  
>>>> also
>>>> something we recognize is a problem for users, we've slated it  
>>>> for 3.3.0
>>>> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>>>>
>>>
>>>
>>>
>

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Hadoop training and consulting
http://www.scaleunlimited.com
http://www.101tec.com




Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
You might want to add a link to zkclient on this page:

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

Patrick

Patrick Hunt wrote:
> Ted Dunning wrote:
>> Judging by history and that fact that only 40/127 issues are resolved, 
>> 3.3
>> is probably 3-6 months away.  Is that a fair assessment?
> 
> Yes, that's fair.
> 
> Patrick
> 
>> On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org> wrote:
>>
>>> One nice thing about ephemeral is that the Stat contains the owner
>>> sessionid. As you say, it's highly implementation dependent. It's also
>>> something we recognize is a problem for users, we've slated it for 3.3.0
>>> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>>>
>>
>>
>>

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
You might want to add a link to zkclient on this page:

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

Patrick

Patrick Hunt wrote:
> Ted Dunning wrote:
>> Judging by history and that fact that only 40/127 issues are resolved, 
>> 3.3
>> is probably 3-6 months away.  Is that a fair assessment?
> 
> Yes, that's fair.
> 
> Patrick
> 
>> On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org> wrote:
>>
>>> One nice thing about ephemeral is that the Stat contains the owner
>>> sessionid. As you say, it's highly implementation dependent. It's also
>>> something we recognize is a problem for users, we've slated it for 3.3.0
>>> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>>>
>>
>>
>>

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Ted Dunning wrote:
> Judging by history and that fact that only 40/127 issues are resolved, 3.3
> is probably 3-6 months away.  Is that a fair assessment?

Yes, that's fair.

Patrick

> On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org> wrote:
> 
>> One nice thing about ephemeral is that the Stat contains the owner
>> sessionid. As you say, it's highly implementation dependent. It's also
>> something we recognize is a problem for users, we've slated it for 3.3.0
>> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>>
> 
> 
> 

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Ted Dunning wrote:
> Judging by history and that fact that only 40/127 issues are resolved, 3.3
> is probably 3-6 months away.  Is that a fair assessment?

Yes, that's fair.

Patrick

> On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org> wrote:
> 
>> One nice thing about ephemeral is that the Stat contains the owner
>> sessionid. As you say, it's highly implementation dependent. It's also
>> something we recognize is a problem for users, we've slated it for 3.3.0
>> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>>
> 
> 
> 

Re: feedback zkclient

Posted by Ted Dunning <te...@gmail.com>.
That looks really lovely.

Judging by history and that fact that only 40/127 issues are resolved, 3.3
is probably 3-6 months away.  Is that a fair assessment?

On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org> wrote:

> One nice thing about ephemeral is that the Stat contains the owner
> sessionid. As you say, it's highly implementation dependent. It's also
> something we recognize is a problem for users, we've slated it for 3.3.0
> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>



-- 
Ted Dunning, CTO
DeepDyve

Re: feedback zkclient

Posted by Ted Dunning <te...@gmail.com>.
That looks really lovely.

Judging by history and that fact that only 40/127 issues are resolved, 3.3
is probably 3-6 months away.  Is that a fair assessment?

On Thu, Oct 1, 2009 at 11:13 AM, Patrick Hunt <ph...@apache.org> wrote:

> One nice thing about ephemeral is that the Stat contains the owner
> sessionid. As you say, it's highly implementation dependent. It's also
> something we recognize is a problem for users, we've slated it for 3.3.0
> http://issues.apache.org/jira/browse/ZOOKEEPER-22
>



-- 
Ted Dunning, CTO
DeepDyve

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Ted Dunning wrote:
> You may be able to tell if the file is yours be examining the content and
> ownership, but this is pretty implementation dependent.  In particular, it
> makes queues very difficult to implement correctly.  If this happens during
> the creation of an ephemeral file, the only option may be to close the
> connection (thus deleting all ephemeral files) and start over.

One nice thing about ephemeral is that the Stat contains the owner 
sessionid. As you say, it's highly implementation dependent. It's also 
something we recognize is a problem for users, we've slated it for 3.3.0
http://issues.apache.org/jira/browse/ZOOKEEPER-22

Patrick


> 
> On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:
> 
>> 3) there's definitely an issue in the retryUntilConnected logic that you
>>> need to address
>>>
>>> let's say you call zkclient.create, and the connection to the server is
>>> lost while the request is in flight. At this point ConnectionLoss is thrown
>>> on the client side, however you (client) have no information on whether the
>>> server has made the change or not. The retry method's while loop will re-run
>>> the create (after reconnect), and the result seen by the caller (user code)
>>> could be either OK or may be NODEEXISTS exception, there's no way to know
>>> which.
>>>
>>> Mahadev is working on ZOOKEEPER-22 which will address this issue, but
>>> that's a future version, not today.
>>>
>> Good catch. I wasn't aware that nodes could still be have been created when
>> receiving a ConnectionLoss. But how would you deal with that?
>> If we create a znode and get a ConnectionLoss exception, then wait until
>> the connection is back and check if the znode is there. There is no way of
>> knowing whether it was us who created the node or somebody else, right?
> 
> 
> 
> 

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Ted Dunning wrote:
> You may be able to tell if the file is yours be examining the content and
> ownership, but this is pretty implementation dependent.  In particular, it
> makes queues very difficult to implement correctly.  If this happens during
> the creation of an ephemeral file, the only option may be to close the
> connection (thus deleting all ephemeral files) and start over.

One nice thing about ephemeral is that the Stat contains the owner 
sessionid. As you say, it's highly implementation dependent. It's also 
something we recognize is a problem for users, we've slated it for 3.3.0
http://issues.apache.org/jira/browse/ZOOKEEPER-22

Patrick


> 
> On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:
> 
>> 3) there's definitely an issue in the retryUntilConnected logic that you
>>> need to address
>>>
>>> let's say you call zkclient.create, and the connection to the server is
>>> lost while the request is in flight. At this point ConnectionLoss is thrown
>>> on the client side, however you (client) have no information on whether the
>>> server has made the change or not. The retry method's while loop will re-run
>>> the create (after reconnect), and the result seen by the caller (user code)
>>> could be either OK or may be NODEEXISTS exception, there's no way to know
>>> which.
>>>
>>> Mahadev is working on ZOOKEEPER-22 which will address this issue, but
>>> that's a future version, not today.
>>>
>> Good catch. I wasn't aware that nodes could still be have been created when
>> receiving a ConnectionLoss. But how would you deal with that?
>> If we create a znode and get a ConnectionLoss exception, then wait until
>> the connection is back and check if the znode is there. There is no way of
>> knowing whether it was us who created the node or somebody else, right?
> 
> 
> 
> 

Re: feedback zkclient

Posted by Ted Dunning <te...@gmail.com>.
There is not much way to totally avoid this without massive performance loss
because the connection loss could be during the the time that the
confirmation is returning.

You may be able to tell if the file is yours be examining the content and
ownership, but this is pretty implementation dependent.  In particular, it
makes queues very difficult to implement correctly.  If this happens during
the creation of an ephemeral file, the only option may be to close the
connection (thus deleting all ephemeral files) and start over.

On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:

> 3) there's definitely an issue in the retryUntilConnected logic that you
>> need to address
>>
>> let's say you call zkclient.create, and the connection to the server is
>> lost while the request is in flight. At this point ConnectionLoss is thrown
>> on the client side, however you (client) have no information on whether the
>> server has made the change or not. The retry method's while loop will re-run
>> the create (after reconnect), and the result seen by the caller (user code)
>> could be either OK or may be NODEEXISTS exception, there's no way to know
>> which.
>>
>> Mahadev is working on ZOOKEEPER-22 which will address this issue, but
>> that's a future version, not today.
>>
>
> Good catch. I wasn't aware that nodes could still be have been created when
> receiving a ConnectionLoss. But how would you deal with that?
> If we create a znode and get a ConnectionLoss exception, then wait until
> the connection is back and check if the znode is there. There is no way of
> knowing whether it was us who created the node or somebody else, right?




-- 
Ted Dunning, CTO
DeepDyve

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Peter Voss wrote:
> On 01.10.2009, at 08:57, Patrick Hunt wrote:
>> 2) what purpose does ZkEventThread serve?
> 
> ZkClient updates it's connection state from the ZooKeeper events. Based 
> on these it notifies listeners, updates it's connection state or 
> reconnects to ZooKeeper. ZkClient has its own event thread to prevent 
> dead-locks. When a listener blocks (because it waits until ZkClient has 
> reconnected to Zookeeper), ZkClient wouldn't be able to receive the 
> reconnect event from ZooKeeper anymore, if we had re-used the Zookeeper 
> event thread to notifier listeners. See the javadoc for ZkEventThread 
> for more information.
> 

I see. Ok, that makes sense. I did see the javadoc on that, but I didn't 
make the connection on what "deadlock" it was talking about. Perhaps it 
was just the late hour and I was too sleepy. ;-) You might want to just 
add this to your existing javadoc to make it more clear.

>> 3) there's definitely an issue in the retryUntilConnected logic that 
>> you need to address

> 
> Good catch. I wasn't aware that nodes could still be have been created 
> when receiving a ConnectionLoss. But how would you deal with that?
> If we create a znode and get a ConnectionLoss exception, then wait until 
> the connection is back and check if the znode is there. There is no way 
> of knowing whether it was us who created the node or somebody else, right?
> Anyway. That's definitely a design issue.
> 

Well this is what makes connloss a problem, we recognize that this is 
not a great situation for users, and why we are doing ZOOKEEPER-22.

Using the std bindings it's really up to the user/situation. In some 
cases it's easy (if you're the only potential owner) in others not so much.

>> 4) when I saw that you had separated zkclient and zkconnection I 
>> thought "ah, this is interesting" however when I saw the 
>> implementation I was confused:
>>
>> a) what purpose does this separation serve?
> 
> It's just to have all ZooKeeper communication in one place, where the 
> higher lever stuff is in ZkClient. That way we are able to provide an 
> in-memory ZkConnection implementation that doesn't connect to a real 
> ZooKeeper. This could be used for easier testing.
> 
>> b) I thought it was to allow multiple zkclients to share a single 
>> connection, however looking at zkclient.close, it closes the 
>> underlying connection.
> 
> Actually each ZkClient instance maintains one ZooKeeper connection.
> 

Ok, I see the illegalstateexception being thrown in zkconnection. gotit. 
(putting this in the javadoc would really help though ;-) )

>> 5) there's a lot of wrapping of exceptions, looks like this is done in 
>> order to make them unchecked. Is this wise? How much "simpler" does it 
>> really make things? Esp things like interrupted exception? As you 
>> mentioned, one of your intents is to simplify things, but perhaps too 
>> simple? Some short, clear examples of usage would be helpful here to 
>> compare/contrast, I took a very quick look at some of the tests but 
>> that didn't help much. Is there a test(s) in particular that I should 
>> look at to see how zkclient is used, and the benefits incurred?
> 
> Checked exceptions are very painful when you are assembling together a 
> larger number of libraries (which is true for most enterprise 
> applications). Either you wind up having a general "throws Exception" 
> (which I don't really like, because it's too general) at most of your 
> interfaces, or you have to wrap checked exceptions into runtime exceptions.
> 
> We didn't want a library to introduce yet another checked exception that 
> you MUST catch or rethrow. I know that there are different opinions 
> about that, but that's the idea behind this.
> 
> Similar situation for the InterruptedException. ZkClient also converts 
> this to a runtime exception and makes sure that the interrupted flag 
> doesn't get cleared. There are just too many existing libraries that 
> have a "catch (Exception e)" somewhere that totally ignores that this 
> would reset the interrupt flag, if e is an InterruptedException. 
> Therefore we better avoid having all of the methods throwing that 
> exception.
> 

Ok. I didn't mean to imply that it's wrong - just that it should be well 
thought out for the particular use cases you want to support and well 
documented for the user. I think sun did the right thing with 
exceptions, having both checked and unchecked lets you tailor the api 
for the use cases you want to support. In too many cases though ppl 
don't think about it (I hate when I see stuff like what you mentioned, 
doing the wrong thing with interrupts for example) and do the wrong 
thing, making the end user suffer.

> Thanks a lot for the valuable feedback,

NP. It's great to see users interested enough in ZK to provide 
contributions back to the community!

Patrick

ps. I post some (interesting) side projects, many related to ZK, on github
http://github.com/phunt
feel free to follow me, or fork some of my projects there.

> --Peter
> 
>>
>> Regards,
>>
>> Patrick
>>
>> Patrick Hunt wrote:
>>> Hi Stefan, two suggestions off the bat:
>>> 1) fill in something in the README, doesn't have to be final or 
>>> polished, but give some insight into the 
>>> what/why/how/where/goals/etc... to get things moving quickly for 
>>> reviewers & new users.
>>> 2) you should really discuss on the dev list. It's up to you to 
>>> include user, but apache discourages use of user for development 
>>> discussion (plus you'll pickup more developer insight there)
>>> Patrick
>>> Stefan Groschupf wrote:
>>>> Hi Zookeeper developer,
>>>> it would be great if you guys could give us some feedback about our 
>>>> project zkclient.
>>>> http://github.com/joa23/zkclient
>>>> The main idea is making the life of lazy developers that only want 
>>>> minimal zk functionality much easier.
>>>>
>>>> We have a functionality like zkclient mock making testing easy and 
>>>> fast without running a real zkserver, simple call back interfaces 
>>>> for the different event types, reconnecting handling in case of 
>>>> timeout etc.
>>>>
>>>> We feel we come closer to a release so it would be great if some 
>>>> experts could have a look and give us some feedback.
>>>> Thanks,
>>>> Stefan
>>>>
>>>>
>>>>
>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> Hadoop training and consulting
>>>> http://www.scaleunlimited.com
>>>> http://www.101tec.com
>>>>
>>>>
>>>>
> 

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Not to harp on this ;-) but this sounds like something that would be a 
very helpful addition to the README.

Ted Dunning wrote:
> I think that another way to say this is that zkClient is going a bit for the
> Spring philosophy that if the caller can't (or won't) be handling the
> situation, then they shouldn't be forced to declare it.  The Spring
> jdbcTemplate is a grand example of the benefits of this.
> 
> First implementations of this policy generally are a bit too broad, though,
> so this should be examined carefully.
> 
> On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:
> 
>> 5) there's a lot of wrapping of exceptions, looks like this is done in
>>> order to make them unchecked. Is this wise? How much "simpler" does it
>>> really make things? Esp things like interrupted exception? As you mentioned,
>>> one of your intents is to simplify things, but perhaps too simple? Some
>>> short, clear examples of usage would be helpful here to compare/contrast, I
>>> took a very quick look at some of the tests but that didn't help much. Is
>>> there a test(s) in particular that I should look at to see how zkclient is
>>> used, and the benefits incurred?
>>>
>> Checked exceptions are very painful when you are assembling together a
>> larger number of libraries (which is true for most enterprise applications).
>> Either you wind up having a general "throws Exception" (which I don't really
>> like, because it's too general) at most of your interfaces, or you have to
>> wrap checked exceptions into runtime exceptions.
>>
>> We didn't want a library to introduce yet another checked exception that
>> you MUST catch or rethrow. I know that there are different opinions about
>> that, but that's the idea behind this.
>>
>> Similar situation for the InterruptedException. ZkClient also converts this
>> to a runtime exception and makes sure that the interrupted flag doesn't get
>> cleared. There are just too many existing libraries that have a "catch
>> (Exception e)" somewhere that totally ignores that this would reset the
>> interrupt flag, if e is an InterruptedException. Therefore we better avoid
>> having all of the methods throwing that exception.
> 
> 
> 
> 

Re: feedback zkclient

Posted by Adam Rosien <ad...@rosien.net>.
I'm on the fence about checked vs. unchecked, but I think Javadoc that
declares what exceptions are thrown would help users figure out if
they might want to catch something.

Additionally some of the ZkClient method semantics are confusing. For
example, create() returns the path of the created node, but throws a
(runtime) ZkNodeExistsException when the node already exists. Creating
a node that already exists is a relatively non-exceptional
circumstance IMHO. Wouldn't it be simpler to return the path to the
client, essentially pretending that the creation succeeded?

Also, is it correct that if you want to "use" a node that may not
exist, you'd have to call create() and then wait for the node to
actually be created via (in zkclient) waitUntilExists()? (Obviously
the node can be removed before it is used, and that situation needs to
be taken care of anyway.) Perhaps create() and waitUntilExists()
should be combined, or offer a combined version. As a naive zk user a
wrapper like that would remove a lot of error potential.

Apologies if this is not the best place for zkclient API discussions.

.. Adam

On Thu, Oct 1, 2009 at 9:19 AM, Ted Dunning <te...@gmail.com> wrote:
> I think that another way to say this is that zkClient is going a bit for the
> Spring philosophy that if the caller can't (or won't) be handling the
> situation, then they shouldn't be forced to declare it.  The Spring
> jdbcTemplate is a grand example of the benefits of this.
>
> First implementations of this policy generally are a bit too broad, though,
> so this should be examined carefully.
>
> On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:
>
>> 5) there's a lot of wrapping of exceptions, looks like this is done in
>>> order to make them unchecked. Is this wise? How much "simpler" does it
>>> really make things? Esp things like interrupted exception? As you mentioned,
>>> one of your intents is to simplify things, but perhaps too simple? Some
>>> short, clear examples of usage would be helpful here to compare/contrast, I
>>> took a very quick look at some of the tests but that didn't help much. Is
>>> there a test(s) in particular that I should look at to see how zkclient is
>>> used, and the benefits incurred?
>>>
>>
>> Checked exceptions are very painful when you are assembling together a
>> larger number of libraries (which is true for most enterprise applications).
>> Either you wind up having a general "throws Exception" (which I don't really
>> like, because it's too general) at most of your interfaces, or you have to
>> wrap checked exceptions into runtime exceptions.
>>
>> We didn't want a library to introduce yet another checked exception that
>> you MUST catch or rethrow. I know that there are different opinions about
>> that, but that's the idea behind this.
>>
>> Similar situation for the InterruptedException. ZkClient also converts this
>> to a runtime exception and makes sure that the interrupted flag doesn't get
>> cleared. There are just too many existing libraries that have a "catch
>> (Exception e)" somewhere that totally ignores that this would reset the
>> interrupt flag, if e is an InterruptedException. Therefore we better avoid
>> having all of the methods throwing that exception.
>
>
>
>
> --
> Ted Dunning, CTO
> DeepDyve
>

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Not to harp on this ;-) but this sounds like something that would be a 
very helpful addition to the README.

Ted Dunning wrote:
> I think that another way to say this is that zkClient is going a bit for the
> Spring philosophy that if the caller can't (or won't) be handling the
> situation, then they shouldn't be forced to declare it.  The Spring
> jdbcTemplate is a grand example of the benefits of this.
> 
> First implementations of this policy generally are a bit too broad, though,
> so this should be examined carefully.
> 
> On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:
> 
>> 5) there's a lot of wrapping of exceptions, looks like this is done in
>>> order to make them unchecked. Is this wise? How much "simpler" does it
>>> really make things? Esp things like interrupted exception? As you mentioned,
>>> one of your intents is to simplify things, but perhaps too simple? Some
>>> short, clear examples of usage would be helpful here to compare/contrast, I
>>> took a very quick look at some of the tests but that didn't help much. Is
>>> there a test(s) in particular that I should look at to see how zkclient is
>>> used, and the benefits incurred?
>>>
>> Checked exceptions are very painful when you are assembling together a
>> larger number of libraries (which is true for most enterprise applications).
>> Either you wind up having a general "throws Exception" (which I don't really
>> like, because it's too general) at most of your interfaces, or you have to
>> wrap checked exceptions into runtime exceptions.
>>
>> We didn't want a library to introduce yet another checked exception that
>> you MUST catch or rethrow. I know that there are different opinions about
>> that, but that's the idea behind this.
>>
>> Similar situation for the InterruptedException. ZkClient also converts this
>> to a runtime exception and makes sure that the interrupted flag doesn't get
>> cleared. There are just too many existing libraries that have a "catch
>> (Exception e)" somewhere that totally ignores that this would reset the
>> interrupt flag, if e is an InterruptedException. Therefore we better avoid
>> having all of the methods throwing that exception.
> 
> 
> 
> 

Re: feedback zkclient

Posted by Ted Dunning <te...@gmail.com>.
I think that another way to say this is that zkClient is going a bit for the
Spring philosophy that if the caller can't (or won't) be handling the
situation, then they shouldn't be forced to declare it.  The Spring
jdbcTemplate is a grand example of the benefits of this.

First implementations of this policy generally are a bit too broad, though,
so this should be examined carefully.

On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:

> 5) there's a lot of wrapping of exceptions, looks like this is done in
>> order to make them unchecked. Is this wise? How much "simpler" does it
>> really make things? Esp things like interrupted exception? As you mentioned,
>> one of your intents is to simplify things, but perhaps too simple? Some
>> short, clear examples of usage would be helpful here to compare/contrast, I
>> took a very quick look at some of the tests but that didn't help much. Is
>> there a test(s) in particular that I should look at to see how zkclient is
>> used, and the benefits incurred?
>>
>
> Checked exceptions are very painful when you are assembling together a
> larger number of libraries (which is true for most enterprise applications).
> Either you wind up having a general "throws Exception" (which I don't really
> like, because it's too general) at most of your interfaces, or you have to
> wrap checked exceptions into runtime exceptions.
>
> We didn't want a library to introduce yet another checked exception that
> you MUST catch or rethrow. I know that there are different opinions about
> that, but that's the idea behind this.
>
> Similar situation for the InterruptedException. ZkClient also converts this
> to a runtime exception and makes sure that the interrupted flag doesn't get
> cleared. There are just too many existing libraries that have a "catch
> (Exception e)" somewhere that totally ignores that this would reset the
> interrupt flag, if e is an InterruptedException. Therefore we better avoid
> having all of the methods throwing that exception.




-- 
Ted Dunning, CTO
DeepDyve

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
Peter Voss wrote:
> On 01.10.2009, at 08:57, Patrick Hunt wrote:
>> 2) what purpose does ZkEventThread serve?
> 
> ZkClient updates it's connection state from the ZooKeeper events. Based 
> on these it notifies listeners, updates it's connection state or 
> reconnects to ZooKeeper. ZkClient has its own event thread to prevent 
> dead-locks. When a listener blocks (because it waits until ZkClient has 
> reconnected to Zookeeper), ZkClient wouldn't be able to receive the 
> reconnect event from ZooKeeper anymore, if we had re-used the Zookeeper 
> event thread to notifier listeners. See the javadoc for ZkEventThread 
> for more information.
> 

I see. Ok, that makes sense. I did see the javadoc on that, but I didn't 
make the connection on what "deadlock" it was talking about. Perhaps it 
was just the late hour and I was too sleepy. ;-) You might want to just 
add this to your existing javadoc to make it more clear.

>> 3) there's definitely an issue in the retryUntilConnected logic that 
>> you need to address

> 
> Good catch. I wasn't aware that nodes could still be have been created 
> when receiving a ConnectionLoss. But how would you deal with that?
> If we create a znode and get a ConnectionLoss exception, then wait until 
> the connection is back and check if the znode is there. There is no way 
> of knowing whether it was us who created the node or somebody else, right?
> Anyway. That's definitely a design issue.
> 

Well this is what makes connloss a problem, we recognize that this is 
not a great situation for users, and why we are doing ZOOKEEPER-22.

Using the std bindings it's really up to the user/situation. In some 
cases it's easy (if you're the only potential owner) in others not so much.

>> 4) when I saw that you had separated zkclient and zkconnection I 
>> thought "ah, this is interesting" however when I saw the 
>> implementation I was confused:
>>
>> a) what purpose does this separation serve?
> 
> It's just to have all ZooKeeper communication in one place, where the 
> higher lever stuff is in ZkClient. That way we are able to provide an 
> in-memory ZkConnection implementation that doesn't connect to a real 
> ZooKeeper. This could be used for easier testing.
> 
>> b) I thought it was to allow multiple zkclients to share a single 
>> connection, however looking at zkclient.close, it closes the 
>> underlying connection.
> 
> Actually each ZkClient instance maintains one ZooKeeper connection.
> 

Ok, I see the illegalstateexception being thrown in zkconnection. gotit. 
(putting this in the javadoc would really help though ;-) )

>> 5) there's a lot of wrapping of exceptions, looks like this is done in 
>> order to make them unchecked. Is this wise? How much "simpler" does it 
>> really make things? Esp things like interrupted exception? As you 
>> mentioned, one of your intents is to simplify things, but perhaps too 
>> simple? Some short, clear examples of usage would be helpful here to 
>> compare/contrast, I took a very quick look at some of the tests but 
>> that didn't help much. Is there a test(s) in particular that I should 
>> look at to see how zkclient is used, and the benefits incurred?
> 
> Checked exceptions are very painful when you are assembling together a 
> larger number of libraries (which is true for most enterprise 
> applications). Either you wind up having a general "throws Exception" 
> (which I don't really like, because it's too general) at most of your 
> interfaces, or you have to wrap checked exceptions into runtime exceptions.
> 
> We didn't want a library to introduce yet another checked exception that 
> you MUST catch or rethrow. I know that there are different opinions 
> about that, but that's the idea behind this.
> 
> Similar situation for the InterruptedException. ZkClient also converts 
> this to a runtime exception and makes sure that the interrupted flag 
> doesn't get cleared. There are just too many existing libraries that 
> have a "catch (Exception e)" somewhere that totally ignores that this 
> would reset the interrupt flag, if e is an InterruptedException. 
> Therefore we better avoid having all of the methods throwing that 
> exception.
> 

Ok. I didn't mean to imply that it's wrong - just that it should be well 
thought out for the particular use cases you want to support and well 
documented for the user. I think sun did the right thing with 
exceptions, having both checked and unchecked lets you tailor the api 
for the use cases you want to support. In too many cases though ppl 
don't think about it (I hate when I see stuff like what you mentioned, 
doing the wrong thing with interrupts for example) and do the wrong 
thing, making the end user suffer.

> Thanks a lot for the valuable feedback,

NP. It's great to see users interested enough in ZK to provide 
contributions back to the community!

Patrick

ps. I post some (interesting) side projects, many related to ZK, on github
http://github.com/phunt
feel free to follow me, or fork some of my projects there.

> --Peter
> 
>>
>> Regards,
>>
>> Patrick
>>
>> Patrick Hunt wrote:
>>> Hi Stefan, two suggestions off the bat:
>>> 1) fill in something in the README, doesn't have to be final or 
>>> polished, but give some insight into the 
>>> what/why/how/where/goals/etc... to get things moving quickly for 
>>> reviewers & new users.
>>> 2) you should really discuss on the dev list. It's up to you to 
>>> include user, but apache discourages use of user for development 
>>> discussion (plus you'll pickup more developer insight there)
>>> Patrick
>>> Stefan Groschupf wrote:
>>>> Hi Zookeeper developer,
>>>> it would be great if you guys could give us some feedback about our 
>>>> project zkclient.
>>>> http://github.com/joa23/zkclient
>>>> The main idea is making the life of lazy developers that only want 
>>>> minimal zk functionality much easier.
>>>>
>>>> We have a functionality like zkclient mock making testing easy and 
>>>> fast without running a real zkserver, simple call back interfaces 
>>>> for the different event types, reconnecting handling in case of 
>>>> timeout etc.
>>>>
>>>> We feel we come closer to a release so it would be great if some 
>>>> experts could have a look and give us some feedback.
>>>> Thanks,
>>>> Stefan
>>>>
>>>>
>>>>
>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> Hadoop training and consulting
>>>> http://www.scaleunlimited.com
>>>> http://www.101tec.com
>>>>
>>>>
>>>>
> 

Re: feedback zkclient

Posted by Ted Dunning <te...@gmail.com>.
There is not much way to totally avoid this without massive performance loss
because the connection loss could be during the the time that the
confirmation is returning.

You may be able to tell if the file is yours be examining the content and
ownership, but this is pretty implementation dependent.  In particular, it
makes queues very difficult to implement correctly.  If this happens during
the creation of an ephemeral file, the only option may be to close the
connection (thus deleting all ephemeral files) and start over.

On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:

> 3) there's definitely an issue in the retryUntilConnected logic that you
>> need to address
>>
>> let's say you call zkclient.create, and the connection to the server is
>> lost while the request is in flight. At this point ConnectionLoss is thrown
>> on the client side, however you (client) have no information on whether the
>> server has made the change or not. The retry method's while loop will re-run
>> the create (after reconnect), and the result seen by the caller (user code)
>> could be either OK or may be NODEEXISTS exception, there's no way to know
>> which.
>>
>> Mahadev is working on ZOOKEEPER-22 which will address this issue, but
>> that's a future version, not today.
>>
>
> Good catch. I wasn't aware that nodes could still be have been created when
> receiving a ConnectionLoss. But how would you deal with that?
> If we create a znode and get a ConnectionLoss exception, then wait until
> the connection is back and check if the znode is there. There is no way of
> knowing whether it was us who created the node or somebody else, right?




-- 
Ted Dunning, CTO
DeepDyve

Re: feedback zkclient

Posted by Ted Dunning <te...@gmail.com>.
I think that another way to say this is that zkClient is going a bit for the
Spring philosophy that if the caller can't (or won't) be handling the
situation, then they shouldn't be forced to declare it.  The Spring
jdbcTemplate is a grand example of the benefits of this.

First implementations of this policy generally are a bit too broad, though,
so this should be examined carefully.

On Thu, Oct 1, 2009 at 8:05 AM, Peter Voss <in...@petervoss.org> wrote:

> 5) there's a lot of wrapping of exceptions, looks like this is done in
>> order to make them unchecked. Is this wise? How much "simpler" does it
>> really make things? Esp things like interrupted exception? As you mentioned,
>> one of your intents is to simplify things, but perhaps too simple? Some
>> short, clear examples of usage would be helpful here to compare/contrast, I
>> took a very quick look at some of the tests but that didn't help much. Is
>> there a test(s) in particular that I should look at to see how zkclient is
>> used, and the benefits incurred?
>>
>
> Checked exceptions are very painful when you are assembling together a
> larger number of libraries (which is true for most enterprise applications).
> Either you wind up having a general "throws Exception" (which I don't really
> like, because it's too general) at most of your interfaces, or you have to
> wrap checked exceptions into runtime exceptions.
>
> We didn't want a library to introduce yet another checked exception that
> you MUST catch or rethrow. I know that there are different opinions about
> that, but that's the idea behind this.
>
> Similar situation for the InterruptedException. ZkClient also converts this
> to a runtime exception and makes sure that the interrupted flag doesn't get
> cleared. There are just too many existing libraries that have a "catch
> (Exception e)" somewhere that totally ignores that this would reset the
> interrupt flag, if e is an InterruptedException. Therefore we better avoid
> having all of the methods throwing that exception.




-- 
Ted Dunning, CTO
DeepDyve

Re: feedback zkclient

Posted by Peter Voss <in...@petervoss.org>.
Hi Patrick,

On 01.10.2009, at 08:57, Patrick Hunt wrote:

> I started looking a bit more closely at the source, some questions:
>
> 1) I tried generating the javadocs (see my fork of the project on  
> github if you want my changes to build.xml for this) but it looks  
> like there's pretty much no javadoc. Some information, particularly  
> on semantics of user-exposed operations would be useful (esp re my  
> earlier README comment - some high level document describing the  
> benefits, etc... of the library)
>
> If I'm your proto-typical "lazy developer" (which I am :-) ), I'm  
> really expecting some helpful docs to get me bootstrapped.
>
> 2) what purpose does ZkEventThread serve?

ZkClient updates it's connection state from the ZooKeeper events.  
Based on these it notifies listeners, updates it's connection state or  
reconnects to ZooKeeper. ZkClient has its own event thread to prevent  
dead-locks. When a listener blocks (because it waits until ZkClient  
has reconnected to Zookeeper), ZkClient wouldn't be able to receive  
the reconnect event from ZooKeeper anymore, if we had re-used the  
Zookeeper event thread to notifier listeners. See the javadoc for  
ZkEventThread for more information.

> 3) there's definitely an issue in the retryUntilConnected logic that  
> you need to address
>
> let's say you call zkclient.create, and the connection to the server  
> is lost while the request is in flight. At this point ConnectionLoss  
> is thrown on the client side, however you (client) have no  
> information on whether the server has made the change or not. The  
> retry method's while loop will re-run the create (after reconnect),  
> and the result seen by the caller (user code) could be either OK or  
> may be NODEEXISTS exception, there's no way to know which.
>
> Mahadev is working on ZOOKEEPER-22 which will address this issue,  
> but that's a future version, not today.

Good catch. I wasn't aware that nodes could still be have been created  
when receiving a ConnectionLoss. But how would you deal with that?
If we create a znode and get a ConnectionLoss exception, then wait  
until the connection is back and check if the znode is there. There is  
no way of knowing whether it was us who created the node or somebody  
else, right?
Anyway. That's definitely a design issue.

> 4) when I saw that you had separated zkclient and zkconnection I  
> thought "ah, this is interesting" however when I saw the  
> implementation I was confused:
>
> a) what purpose does this separation serve?

It's just to have all ZooKeeper communication in one place, where the  
higher lever stuff is in ZkClient. That way we are able to provide an  
in-memory ZkConnection implementation that doesn't connect to a real  
ZooKeeper. This could be used for easier testing.

> b) I thought it was to allow multiple zkclients to share a single  
> connection, however looking at zkclient.close, it closes the  
> underlying connection.

Actually each ZkClient instance maintains one ZooKeeper connection.

> 5) there's a lot of wrapping of exceptions, looks like this is done  
> in order to make them unchecked. Is this wise? How much "simpler"  
> does it really make things? Esp things like interrupted exception?  
> As you mentioned, one of your intents is to simplify things, but  
> perhaps too simple? Some short, clear examples of usage would be  
> helpful here to compare/contrast, I took a very quick look at some  
> of the tests but that didn't help much. Is there a test(s) in  
> particular that I should look at to see how zkclient is used, and  
> the benefits incurred?

Checked exceptions are very painful when you are assembling together a  
larger number of libraries (which is true for most enterprise  
applications). Either you wind up having a general "throws  
Exception" (which I don't really like, because it's too general) at  
most of your interfaces, or you have to wrap checked exceptions into  
runtime exceptions.

We didn't want a library to introduce yet another checked exception  
that you MUST catch or rethrow. I know that there are different  
opinions about that, but that's the idea behind this.

Similar situation for the InterruptedException. ZkClient also converts  
this to a runtime exception and makes sure that the interrupted flag  
doesn't get cleared. There are just too many existing libraries that  
have a "catch (Exception e)" somewhere that totally ignores that this  
would reset the interrupt flag, if e is an InterruptedException.  
Therefore we better avoid having all of the methods throwing that  
exception.

Thanks a lot for the valuable feedback,
--Peter

>
> Regards,
>
> Patrick
>
> Patrick Hunt wrote:
>> Hi Stefan, two suggestions off the bat:
>> 1) fill in something in the README, doesn't have to be final or  
>> polished, but give some insight into the what/why/how/where/goals/ 
>> etc... to get things moving quickly for reviewers & new users.
>> 2) you should really discuss on the dev list. It's up to you to  
>> include user, but apache discourages use of user for development  
>> discussion (plus you'll pickup more developer insight there)
>> Patrick
>> Stefan Groschupf wrote:
>>> Hi Zookeeper developer,
>>> it would be great if you guys could give us some feedback about  
>>> our project zkclient.
>>> http://github.com/joa23/zkclient
>>> The main idea is making the life of lazy developers that only want  
>>> minimal zk functionality much easier.
>>>
>>> We have a functionality like zkclient mock making testing easy and  
>>> fast without running a real zkserver, simple call back interfaces  
>>> for the different event types, reconnecting handling in case of  
>>> timeout etc.
>>>
>>> We feel we come closer to a release so it would be great if some  
>>> experts could have a look and give us some feedback.
>>> Thanks,
>>> Stefan
>>>
>>>
>>>
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> Hadoop training and consulting
>>> http://www.scaleunlimited.com
>>> http://www.101tec.com
>>>
>>>
>>>


Re: feedback zkclient

Posted by Peter Voss <in...@petervoss.org>.
Hi Patrick,

On 01.10.2009, at 08:57, Patrick Hunt wrote:

> I started looking a bit more closely at the source, some questions:
>
> 1) I tried generating the javadocs (see my fork of the project on  
> github if you want my changes to build.xml for this) but it looks  
> like there's pretty much no javadoc. Some information, particularly  
> on semantics of user-exposed operations would be useful (esp re my  
> earlier README comment - some high level document describing the  
> benefits, etc... of the library)
>
> If I'm your proto-typical "lazy developer" (which I am :-) ), I'm  
> really expecting some helpful docs to get me bootstrapped.
>
> 2) what purpose does ZkEventThread serve?

ZkClient updates it's connection state from the ZooKeeper events.  
Based on these it notifies listeners, updates it's connection state or  
reconnects to ZooKeeper. ZkClient has its own event thread to prevent  
dead-locks. When a listener blocks (because it waits until ZkClient  
has reconnected to Zookeeper), ZkClient wouldn't be able to receive  
the reconnect event from ZooKeeper anymore, if we had re-used the  
Zookeeper event thread to notifier listeners. See the javadoc for  
ZkEventThread for more information.

> 3) there's definitely an issue in the retryUntilConnected logic that  
> you need to address
>
> let's say you call zkclient.create, and the connection to the server  
> is lost while the request is in flight. At this point ConnectionLoss  
> is thrown on the client side, however you (client) have no  
> information on whether the server has made the change or not. The  
> retry method's while loop will re-run the create (after reconnect),  
> and the result seen by the caller (user code) could be either OK or  
> may be NODEEXISTS exception, there's no way to know which.
>
> Mahadev is working on ZOOKEEPER-22 which will address this issue,  
> but that's a future version, not today.

Good catch. I wasn't aware that nodes could still be have been created  
when receiving a ConnectionLoss. But how would you deal with that?
If we create a znode and get a ConnectionLoss exception, then wait  
until the connection is back and check if the znode is there. There is  
no way of knowing whether it was us who created the node or somebody  
else, right?
Anyway. That's definitely a design issue.

> 4) when I saw that you had separated zkclient and zkconnection I  
> thought "ah, this is interesting" however when I saw the  
> implementation I was confused:
>
> a) what purpose does this separation serve?

It's just to have all ZooKeeper communication in one place, where the  
higher lever stuff is in ZkClient. That way we are able to provide an  
in-memory ZkConnection implementation that doesn't connect to a real  
ZooKeeper. This could be used for easier testing.

> b) I thought it was to allow multiple zkclients to share a single  
> connection, however looking at zkclient.close, it closes the  
> underlying connection.

Actually each ZkClient instance maintains one ZooKeeper connection.

> 5) there's a lot of wrapping of exceptions, looks like this is done  
> in order to make them unchecked. Is this wise? How much "simpler"  
> does it really make things? Esp things like interrupted exception?  
> As you mentioned, one of your intents is to simplify things, but  
> perhaps too simple? Some short, clear examples of usage would be  
> helpful here to compare/contrast, I took a very quick look at some  
> of the tests but that didn't help much. Is there a test(s) in  
> particular that I should look at to see how zkclient is used, and  
> the benefits incurred?

Checked exceptions are very painful when you are assembling together a  
larger number of libraries (which is true for most enterprise  
applications). Either you wind up having a general "throws  
Exception" (which I don't really like, because it's too general) at  
most of your interfaces, or you have to wrap checked exceptions into  
runtime exceptions.

We didn't want a library to introduce yet another checked exception  
that you MUST catch or rethrow. I know that there are different  
opinions about that, but that's the idea behind this.

Similar situation for the InterruptedException. ZkClient also converts  
this to a runtime exception and makes sure that the interrupted flag  
doesn't get cleared. There are just too many existing libraries that  
have a "catch (Exception e)" somewhere that totally ignores that this  
would reset the interrupt flag, if e is an InterruptedException.  
Therefore we better avoid having all of the methods throwing that  
exception.

Thanks a lot for the valuable feedback,
--Peter

>
> Regards,
>
> Patrick
>
> Patrick Hunt wrote:
>> Hi Stefan, two suggestions off the bat:
>> 1) fill in something in the README, doesn't have to be final or  
>> polished, but give some insight into the what/why/how/where/goals/ 
>> etc... to get things moving quickly for reviewers & new users.
>> 2) you should really discuss on the dev list. It's up to you to  
>> include user, but apache discourages use of user for development  
>> discussion (plus you'll pickup more developer insight there)
>> Patrick
>> Stefan Groschupf wrote:
>>> Hi Zookeeper developer,
>>> it would be great if you guys could give us some feedback about  
>>> our project zkclient.
>>> http://github.com/joa23/zkclient
>>> The main idea is making the life of lazy developers that only want  
>>> minimal zk functionality much easier.
>>>
>>> We have a functionality like zkclient mock making testing easy and  
>>> fast without running a real zkserver, simple call back interfaces  
>>> for the different event types, reconnecting handling in case of  
>>> timeout etc.
>>>
>>> We feel we come closer to a release so it would be great if some  
>>> experts could have a look and give us some feedback.
>>> Thanks,
>>> Stefan
>>>
>>>
>>>
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> Hadoop training and consulting
>>> http://www.scaleunlimited.com
>>> http://www.101tec.com
>>>
>>>
>>>


Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
I started looking a bit more closely at the source, some questions:

1) I tried generating the javadocs (see my fork of the project on github 
if you want my changes to build.xml for this) but it looks like there's 
pretty much no javadoc. Some information, particularly on semantics of 
user-exposed operations would be useful (esp re my earlier README 
comment - some high level document describing the benefits, etc... of 
the library)

If I'm your proto-typical "lazy developer" (which I am :-) ), I'm really 
expecting some helpful docs to get me bootstrapped.

2) what purpose does ZkEventThread serve?

3) there's definitely an issue in the retryUntilConnected logic that you 
need to address

let's say you call zkclient.create, and the connection to the server is 
lost while the request is in flight. At this point ConnectionLoss is 
thrown on the client side, however you (client) have no information on 
whether the server has made the change or not. The retry method's while 
loop will re-run the create (after reconnect), and the result seen by 
the caller (user code) could be either OK or may be NODEEXISTS 
exception, there's no way to know which.

Mahadev is working on ZOOKEEPER-22 which will address this issue, but 
that's a future version, not today.

4) when I saw that you had separated zkclient and zkconnection I thought 
"ah, this is interesting" however when I saw the implementation I was 
confused:

a) what purpose does this separation serve?
b) I thought it was to allow multiple zkclients to share a single 
connection, however looking at zkclient.close, it closes the underlying 
connection.

5) there's a lot of wrapping of exceptions, looks like this is done in 
order to make them unchecked. Is this wise? How much "simpler" does it 
really make things? Esp things like interrupted exception? As you 
mentioned, one of your intents is to simplify things, but perhaps too 
simple? Some short, clear examples of usage would be helpful here to 
compare/contrast, I took a very quick look at some of the tests but that 
didn't help much. Is there a test(s) in particular that I should look at 
to see how zkclient is used, and the benefits incurred?

Regards,

Patrick

Patrick Hunt wrote:
> Hi Stefan, two suggestions off the bat:
> 
> 1) fill in something in the README, doesn't have to be final or 
> polished, but give some insight into the what/why/how/where/goals/etc... 
> to get things moving quickly for reviewers & new users.
> 
> 2) you should really discuss on the dev list. It's up to you to include 
> user, but apache discourages use of user for development discussion 
> (plus you'll pickup more developer insight there)
> 
> Patrick
> 
> Stefan Groschupf wrote:
>> Hi Zookeeper developer,
>> it would be great if you guys could give us some feedback about our 
>> project zkclient.
>> http://github.com/joa23/zkclient
>> The main idea is making the life of lazy developers that only want 
>> minimal zk functionality much easier.
>>
>> We have a functionality like zkclient mock making testing easy and 
>> fast without running a real zkserver, simple call back interfaces for 
>> the different event types, reconnecting handling in case of timeout etc.
>>
>> We feel we come closer to a release so it would be great if some 
>> experts could have a look and give us some feedback.
>> Thanks,
>> Stefan
>>
>>
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> Hadoop training and consulting
>> http://www.scaleunlimited.com
>> http://www.101tec.com
>>
>>
>>

Re: feedback zkclient

Posted by Patrick Hunt <ph...@apache.org>.
I started looking a bit more closely at the source, some questions:

1) I tried generating the javadocs (see my fork of the project on github 
if you want my changes to build.xml for this) but it looks like there's 
pretty much no javadoc. Some information, particularly on semantics of 
user-exposed operations would be useful (esp re my earlier README 
comment - some high level document describing the benefits, etc... of 
the library)

If I'm your proto-typical "lazy developer" (which I am :-) ), I'm really 
expecting some helpful docs to get me bootstrapped.

2) what purpose does ZkEventThread serve?

3) there's definitely an issue in the retryUntilConnected logic that you 
need to address

let's say you call zkclient.create, and the connection to the server is 
lost while the request is in flight. At this point ConnectionLoss is 
thrown on the client side, however you (client) have no information on 
whether the server has made the change or not. The retry method's while 
loop will re-run the create (after reconnect), and the result seen by 
the caller (user code) could be either OK or may be NODEEXISTS 
exception, there's no way to know which.

Mahadev is working on ZOOKEEPER-22 which will address this issue, but 
that's a future version, not today.

4) when I saw that you had separated zkclient and zkconnection I thought 
"ah, this is interesting" however when I saw the implementation I was 
confused:

a) what purpose does this separation serve?
b) I thought it was to allow multiple zkclients to share a single 
connection, however looking at zkclient.close, it closes the underlying 
connection.

5) there's a lot of wrapping of exceptions, looks like this is done in 
order to make them unchecked. Is this wise? How much "simpler" does it 
really make things? Esp things like interrupted exception? As you 
mentioned, one of your intents is to simplify things, but perhaps too 
simple? Some short, clear examples of usage would be helpful here to 
compare/contrast, I took a very quick look at some of the tests but that 
didn't help much. Is there a test(s) in particular that I should look at 
to see how zkclient is used, and the benefits incurred?

Regards,

Patrick

Patrick Hunt wrote:
> Hi Stefan, two suggestions off the bat:
> 
> 1) fill in something in the README, doesn't have to be final or 
> polished, but give some insight into the what/why/how/where/goals/etc... 
> to get things moving quickly for reviewers & new users.
> 
> 2) you should really discuss on the dev list. It's up to you to include 
> user, but apache discourages use of user for development discussion 
> (plus you'll pickup more developer insight there)
> 
> Patrick
> 
> Stefan Groschupf wrote:
>> Hi Zookeeper developer,
>> it would be great if you guys could give us some feedback about our 
>> project zkclient.
>> http://github.com/joa23/zkclient
>> The main idea is making the life of lazy developers that only want 
>> minimal zk functionality much easier.
>>
>> We have a functionality like zkclient mock making testing easy and 
>> fast without running a real zkserver, simple call back interfaces for 
>> the different event types, reconnecting handling in case of timeout etc.
>>
>> We feel we come closer to a release so it would be great if some 
>> experts could have a look and give us some feedback.
>> Thanks,
>> Stefan
>>
>>
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> Hadoop training and consulting
>> http://www.scaleunlimited.com
>> http://www.101tec.com
>>
>>
>>