zookeeper-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patrick Hunt <ph...@apache.org>
Subject Re: feedback zkclient
Date Thu, 01 Oct 2009 18:04:14 GMT
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!


ps. I post some (interesting) side projects, many related to ZK, on github
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

View raw message