Menu
▾
▴
zookeeper-developer — Mailing list to discuss development of zookeeper
You can subscribe to this list here.
| 2007 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
(37) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2008 |
Jan
(27) |
Feb
(34) |
Mar
(30) |
Apr
(151) |
May
(184) |
Jun
(55) |
Jul
(2) |
Aug
(6) |
Sep
|
Oct
(1) |
Nov
|
Dec
|
|
From: SourceForge.net <no...@so...> - 2008-06-10 21:11:54
|
Bugs item #1941120, was opened at 2008-04-12 16:05 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1941120&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Nobody/Anonymous (nobody) Assigned to: Nobody/Anonymous (nobody) Summary: getChildren doesn't return a list of paths Initial Comment: It may or may not be a good thing, but getChildren returns a list of names that have to be combined with the name of the parent in order to get paths. At the least the documentation should say this. It is unlikely to be possible to change teh API, but getChildrenPaths might be a good addition. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-10 14:12 Message: Logged In: YES user_id=12853 Originator: NO Moved to Apache https://issues.apache.org/jira/browse/ZOOKEEPER-12 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1941120&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 21:10:33
|
Bugs item #1941109, was opened at 2008-04-12 15:46 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1941109&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: java client Group: 3.0.0 >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Nobody/Anonymous (nobody) Assigned to: Patrick Hunt (phunt) Summary: ArrayList is used instead of List Initial Comment: Especially in the stuff to do with ACL's, the return type is often ArrayList instead of List. This makes lots of code much more complex than necessary. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-10 14:10 Message: Logged In: YES user_id=12853 Originator: NO Moved to Apache https://issues.apache.org/jira/browse/ZOOKEEPER-11 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1941109&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 21:09:17
|
Bugs item #1941108, was opened at 2008-04-12 15:45 Message generated for change (Settings changed) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1941108&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Nobody/Anonymous (nobody) Assigned to: Patrick Hunt (phunt) Summary: Bad error message Initial Comment: If you try to delete a non-empty directory, the error code returned (-111) is not decoded correctly. This means that the error message is very hard to understand and is somewhat alarming. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-10 14:09 Message: Logged In: YES user_id=12853 Originator: NO Moved to Apache https://issues.apache.org/jira/browse/ZOOKEEPER-10 ---------------------------------------------------------------------- Comment By: Mahadev Konar (mahadevkonar) Date: 2008-04-23 14:15 Message: Logged In: YES user_id=1926680 Originator: NO Ted already has bug and a patch for it in his exception changes... :) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1941108&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 21:07:51
|
Bugs item #1888835, was opened at 2008-02-07 07:07 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1888835&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Benjamin Reed (breed) Assigned to: Nobody/Anonymous (nobody) Summary: Set socket linger longer for commands Initial Comment: When outputting the result of a command we need to set the linger longer so that the client can get the full output when the output is very long. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-10 14:08 Message: Logged In: YES user_id=12853 Originator: NO Moved to Apache https://issues.apache.org/jira/browse/ZOOKEEPER-9 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1888835&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 21:05:42
|
Bugs item #1886743, was opened at 2008-02-04 20:37 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1886743&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: 3.0.0 >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Nobody/Anonymous (nobody) Assigned to: Mahadev Konar (mahadevkonar) Summary: Stat enchaned to include num of children and size Initial Comment: Enhance Stat returned to include number of children and size of data in node. This is make getting this data much cheaper to implement a fuse module. One round-trip is needed instead of two. The only way to get size now is to call get the node data. The only way to get number of children is to call get children. Being able to get this data via node exists would be nice. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-10 14:05 Message: Logged In: YES user_id=12853 Originator: NO Moved to Apache https://issues.apache.org/jira/browse/ZOOKEEPER-8 ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-12 13:06 Message: Logged In: YES user_id=154690 Originator: NO To fix this we will need two Stat structures: one for the protocol which will include the above information and one for the ondisk/inmemory structure which will not include that information. (We already have the number of children and the size in memory and we don't want to use up extra bytes to store it redundantly.) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1886743&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 21:01:48
|
Bugs item #1831408, was opened at 2007-11-13 14:43 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1831408&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: java client Group: 3.0.0 >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Benjamin Reed (breed) Assigned to: Mahadev Konar (mahadevkonar) Summary: Use enums rather than ints for types and state Initial Comment: There are a couple of places in ZooKeeper that ints are used when we should really be using enums. The most severe example are the types and states for watch events. It is not always clear which is a type and which is a state. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-10 14:01 Message: Logged In: YES user_id=12853 Originator: NO Moved to Apache https://issues.apache.org/jira/browse/ZOOKEEPER-7 ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-12 13:06 Message: Logged In: YES user_id=154690 Originator: YES Bart had expressed intrest in fixing this. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-03-18 08:11 Message: Logged In: YES user_id=154690 Originator: YES Yes please! Contributions are welcome! thanx ben ---------------------------------------------------------------------- Comment By: Don Pinto (dodil) Date: 2008-03-17 21:03 Message: Logged In: YES user_id=1289601 Originator: NO Hi Benjamin, Is it OK for me to work on this ? I am interested to contribute to this project. Thx, Don ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1831408&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 16:28:13
|
Patches item #1989049, was opened at 2008-06-09 19:38 Message generated for change (Comment added) made by fpj You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1989049&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: fpj (fpj) >Assigned to: Andrew Kornev (akornev) Summary: Incompatible client server list Initial Comment: This is a patch for request feature: 1937084. ---------------------------------------------------------------------- >Comment By: fpj (fpj) Date: 2008-06-10 18:28 Message: Logged In: YES user_id=1926444 Originator: YES Andrew, Could you give me some pointers on how to update the C client to make it work? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1989049&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-10 16:20:53
|
Patches item #1970356, was opened at 2008-05-23 13:31 Message generated for change (Comment added) made by fpj You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1970356&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: fpj (fpj) Assigned to: Nobody/Anonymous (nobody) Summary: Change LE default to Fast TCP Initial Comment: This is a very simple change to the default of the leader election implementation to the fast version using TCP. It requires a separate port. ---------------------------------------------------------------------- >Comment By: fpj (fpj) Date: 2008-06-10 18:21 Message: Logged In: YES user_id=1926444 Originator: YES I have fetched the latest release, and run the junit tests. It shows no problem. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1970356&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-09 17:38:41
|
Patches item #1989049, was opened at 2008-06-09 19:38 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1989049&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: fpj (fpj) Assigned to: Nobody/Anonymous (nobody) Summary: Incompatible client server list Initial Comment: This is a patch for request feature: 1937084. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1989049&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 19:05:46
|
Patches item #1963584, was opened at 2008-05-13 22:56 Message generated for change (Settings changed) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: 3.0.0 >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Benjamin Reed (breed) Summary: fix build.xml cobertura test harness Initial Comment: Need to fail if the junit tests fail - missing fail is causing build to be successful even if cobertura fails. (ci is running cobertura and not std test target) ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-06 12:05 Message: Logged In: YES user_id=154690 Originator: NO Committed revision 175. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-06 12:00 Message: Logged In: YES user_id=154690 Originator: NO great. I'll commit then. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-06-06 11:55 Message: Logged In: YES user_id=12853 Originator: YES If the tests pass the build will be successful. If a test fails (throws exception or assertX fails) then build will be unsuccessful. This is regardless of println/logging output. Hudson actually parses the output looking for errors (even if build is listed as successful) - if build is successful but there are failure type messages in the stdout/log then it will list the build as unstable, otw will list as success/failure appropriately. I tested the failure case Nigel was seeing and it addressed the issue. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 18:57 Message: Logged In: YES user_id=154690 Originator: NO Is this really enough? It will still show up as unstable build if there are error messages printed even though it returns successfully. Right? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-16 10:44 Message: Logged In: YES user_id=12853 Originator: YES Ben please merge this asap to address ci issue. Thanks. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 19:05:29
|
Patches item #1963584, was opened at 2008-05-13 22:56 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: 3.0.0 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Benjamin Reed (breed) Summary: fix build.xml cobertura test harness Initial Comment: Need to fail if the junit tests fail - missing fail is causing build to be successful even if cobertura fails. (ci is running cobertura and not std test target) ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-06 12:05 Message: Logged In: YES user_id=154690 Originator: NO Committed revision 175. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-06 12:00 Message: Logged In: YES user_id=154690 Originator: NO great. I'll commit then. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-06-06 11:55 Message: Logged In: YES user_id=12853 Originator: YES If the tests pass the build will be successful. If a test fails (throws exception or assertX fails) then build will be unsuccessful. This is regardless of println/logging output. Hudson actually parses the output looking for errors (even if build is listed as successful) - if build is successful but there are failure type messages in the stdout/log then it will list the build as unstable, otw will list as success/failure appropriately. I tested the failure case Nigel was seeing and it addressed the issue. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 18:57 Message: Logged In: YES user_id=154690 Originator: NO Is this really enough? It will still show up as unstable build if there are error messages printed even though it returns successfully. Right? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-16 10:44 Message: Logged In: YES user_id=12853 Originator: YES Ben please merge this asap to address ci issue. Thanks. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 19:02:20
|
Patches item #1937078, was opened at 2008-04-07 14:21 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1937078&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: java client Group: 3.0.0 >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Benjamin Reed (breed) Assigned to: Benjamin Reed (breed) Summary: Passing a watch object to read requests Initial Comment: When using a libraries for higher order synchronization primitives it gets hard to coordinate interests in watch events. It would be much more convenient if a Watcher object could be passed to the getData, getChildren, or exists methods. ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-06 12:02 Message: Logged In: YES user_id=154690 Originator: YES Committed revision 174. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-06 11:58 Message: Logged In: YES user_id=154690 Originator: YES Excellent work Pat! Thanx. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-06-03 15:11 Message: Logged In: YES user_id=12853 Originator: NO this patch (v4) addresses all of the outstanding issues that were identified and also fixes server bug 1981340. please review and let me know your feedback. Note: I added a bunch of new watcher tests for all of the various sync read operations (exist/getdata/getchildren) for both the legacy and watcher obj registration functions. these tests also verify 1981340. as part of adding the tests I refactored clienttest a bit (extract base). note that I commented out the "sleep(5000)" in the test setup of clientbase. is there a reason why this was there? (works fine for me on all the tests w/o this line). the tests run a whole lot faster if this is commented out - if it needs to be there feel free to add it back. too bad we can't wait() on the the server object... perhaps we should fix this at some point (server does notifyall on state change). File Added: watcherobj4.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-01 08:06 Message: Logged In: YES user_id=154690 Originator: YES Looks good! You are still missing 3 and 4. 3) Since there is currently only one watch, yes all watches are notified of a disconnect. 4) Yes, this is a bug in the server. The current behaviors leave untriggered watches which is not right. I've opened a bug for this: [ 1981340 ] Child watches are not triggered when the node is deleted ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-30 12:40 Message: Logged In: YES user_id=12853 Originator: NO I implemented all of your (ben) suggestions except for item 3. This keeps the behavior as close to the original as possible - the default watcher will be called when not sync'd. I register the watcher in finish now - at the very beginning of the function. Please verify this in particular is correct. File Added: watcherobj3.patch ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-30 10:55 Message: Logged In: YES user_id=12853 Originator: NO regarding: 4) as part of reworking the patch I looked closely at the code for triggering watches and based processwatchevent on that, unless I'm missing something what I have is correct behavior -- here is deletenode code on the server: dataWatches.triggerWatch(path, Event.EventNodeDeleted); childWatches.triggerWatch(parentName.equals("")?"/":parentName, Event.EventNodeChildrenChanged); childwatches get childrenchanged on the parent of path while datawatches gets nodedeleted on the path itself 2) so the assumption is that two sets of code won't register the same watch obj? This seems reasonable, but I will make explicit in the docs 3) is that what happens currently? all watches are triggered on disconnect? Otw I'll incorporate the feedback you provided. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-30 09:40 Message: Logged In: YES user_id=154690 Originator: YES This looks much better. There are still a couple of issues: 1) The XXXWatches hashmaps should be final not volatile 2) The elements of the watch hashmaps should be hashsets, not lists. If the same watch is added twice, we only need to invoke once. 3) In processWatchEvent, on disconnect I think all the watches need to be invoked not cleared. This may not really be an issue since I'll be changing this with the auto reregister of watches. 4) In processWatchEvent, the nodeDeleted event gets delivered to both data and child watchers. (If you are watching for children of a node, and that node goes away we trigger a watch.) 5) In processWatchEvent, the remove from the hashmap needs to be done in a synchronized block. Otherwise, you could have concurrent additions to the Hashset, currently ArrayList, going on. 6) You cannot add the watches in the completion function and the blocks of the synchronous calls. If you do you end up with a race condition: you are interested in a change, it comes in right after you get the response but before you actually process it. They all need to be done in one place: finishPacket(); the next packet will not be processed before finishPacket is complete. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-28 15:17 Message: Logged In: YES user_id=12853 Originator: NO I updated the patch based on feedback on original. File Added: watcherobj2.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 13:31 Message: Logged In: YES user_id=154690 Originator: YES As far as the implementation goes, we can't do it in the clientcnxn EventThread since only async events go there. The easiest thing would be to do it in finishPacket(). You should probably add members in Packet to hold the Watcher object put it in watches in finishPacket(). ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 13:26 Message: Logged In: YES user_id=154690 Originator: YES 'Are you saying that if I call getData(node,true) and exists(node,true) and then a subsequent create(node) happens I will get a single watch event of type datachanged for node? I won't see a "datachanged(node)" as well as a "nodecreated(node)", one for each of the read calls respectively?' If a create(node) succeeds, that means the node did not exists, which means that getData got a NoNode error, which means that no watch was set. So, you will only see a nodecreated event and it would be delivered to the Watcher of the exists() call. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:48 Message: Logged In: YES user_id=12853 Originator: NO What would you suggest regarding async read requests and watcher registration - we wrap the callback with code that registers the watcher then calls the client callback? I think this would be cleaner than embedding into the clientcnxn EventThread code. What do you think? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:40 Message: Logged In: YES user_id=12853 Originator: NO Hm. I re-read the docs and I'm still a bit confused by your statements regarding watch types. Are you saying that if I call getData(node,true) and exists(node,true) and then a subsequent create(node) happens I will get a single watch event of type datachanged for node? I won't see a "datachanged(node)" as well as a "nodecreated(node)", one for each of the read calls respectively? The watches wiki http://zookeeper.wiki.sourceforge.net/ZooKeeperWatches should really be beefed up with more detail on which event types are generated based on write ops. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:07 Message: Logged In: YES user_id=12853 Originator: NO I'll rework the patch. Regarding the c vs java - I only implemented java due to the category being set to "java client". Perhaps we should add a "client API" category? Or maybe create two requests instead of 1 -- would be better for me since I'm much more familiar with java than C, you could assign to someone better able to execute. :-) ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 11:22 Message: Logged In: YES user_id=154690 Originator: YES We do also need to implement this in the C client as well, but for simplicity we should make that a separate patch. We shouldn't add watches to the watches hashmap until we get a successful result back from the server. Otherwise, we might trigger watches for things that the client isn't expecting. There is also a problem with type watch types. In the server we have a watch list for data and child watches. It might be good to keep that same division in the client. (Either way would work though.) a watch on getChildren() can result in a childrenChanged or nodeDeleted watch event. getData() can result in dataChanged or nodeDeleted event. exist() can result in dataChanged, nodeCreated, or nodeDeleted event. getChildren() and getData() will not get nodeCreated events, because non existence is an error condition for those two calls and a watch will not be set. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-22 14:09 Message: Logged In: YES user_id=12853 Originator: NO Here's the patch, assigning to Ben for further review. Please pay particular attention to the handling of state changes - I clear the watch table when we are not sync'd to the server. Verify my workflow. File Added: watcherobj.patch ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-20 23:18 Message: Logged In: YES user_id=12853 Originator: NO I've implemented this for Java. I overloaded the respective methods with signatures that take watcher object instead of watch boolean. I added new methods rather than replacing -- this maintains b/w compatibility. 1) should I mark the old methods as deprecated or should we just support both method types (newer signature w/ explicit watcher object && old watch boolean which directs to the "global" watcher.) 2) I see this is listed as category "java client" so I'm assuming there is no need for this feature in c code interface. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-20 17:52 Message: Logged In: YES user_id=12853 Originator: NO I was specifically referring to the client api, not the protocol -- should I assume that that last watcher on a particular node "wins". What I mean is if a client calls "getChildren(path, watcherobj1); getChildren(path, watcherobj2);" (notice same patch) what should the client lib do? I'm assuming use the last watcher, but wanted to verify. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 19:05 Message: Logged In: YES user_id=154690 Originator: YES The client will need to track the multiple watcher objects to invoke when it gets the callback. The client server protocol doesn't change, this is just a client side thing. We should add the ability to remove watches as a separate feature request. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-14 00:21 Message: Logged In: YES user_id=12853 Originator: NO What should we do if the user calls getData multiple times with different watcher objects? Remove previous watch and replace with new watcher, throw error, or allow multiple watchers on particular node? How about removing a watch by calling method on particular node with null watcher? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1937078&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 19:00:15
|
Patches item #1963584, was opened at 2008-05-13 22:56 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: 3.0.0 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Benjamin Reed (breed) Summary: fix build.xml cobertura test harness Initial Comment: Need to fail if the junit tests fail - missing fail is causing build to be successful even if cobertura fails. (ci is running cobertura and not std test target) ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-06 12:00 Message: Logged In: YES user_id=154690 Originator: NO great. I'll commit then. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-06-06 11:55 Message: Logged In: YES user_id=12853 Originator: YES If the tests pass the build will be successful. If a test fails (throws exception or assertX fails) then build will be unsuccessful. This is regardless of println/logging output. Hudson actually parses the output looking for errors (even if build is listed as successful) - if build is successful but there are failure type messages in the stdout/log then it will list the build as unstable, otw will list as success/failure appropriately. I tested the failure case Nigel was seeing and it addressed the issue. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 18:57 Message: Logged In: YES user_id=154690 Originator: NO Is this really enough? It will still show up as unstable build if there are error messages printed even though it returns successfully. Right? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-16 10:44 Message: Logged In: YES user_id=12853 Originator: YES Ben please merge this asap to address ci issue. Thanks. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 18:58:28
|
Patches item #1937078, was opened at 2008-04-07 14:21 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1937078&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: java client Group: 3.0.0 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Benjamin Reed (breed) Assigned to: Benjamin Reed (breed) Summary: Passing a watch object to read requests Initial Comment: When using a libraries for higher order synchronization primitives it gets hard to coordinate interests in watch events. It would be much more convenient if a Watcher object could be passed to the getData, getChildren, or exists methods. ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-06 11:58 Message: Logged In: YES user_id=154690 Originator: YES Excellent work Pat! Thanx. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-06-03 15:11 Message: Logged In: YES user_id=12853 Originator: NO this patch (v4) addresses all of the outstanding issues that were identified and also fixes server bug 1981340. please review and let me know your feedback. Note: I added a bunch of new watcher tests for all of the various sync read operations (exist/getdata/getchildren) for both the legacy and watcher obj registration functions. these tests also verify 1981340. as part of adding the tests I refactored clienttest a bit (extract base). note that I commented out the "sleep(5000)" in the test setup of clientbase. is there a reason why this was there? (works fine for me on all the tests w/o this line). the tests run a whole lot faster if this is commented out - if it needs to be there feel free to add it back. too bad we can't wait() on the the server object... perhaps we should fix this at some point (server does notifyall on state change). File Added: watcherobj4.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-01 08:06 Message: Logged In: YES user_id=154690 Originator: YES Looks good! You are still missing 3 and 4. 3) Since there is currently only one watch, yes all watches are notified of a disconnect. 4) Yes, this is a bug in the server. The current behaviors leave untriggered watches which is not right. I've opened a bug for this: [ 1981340 ] Child watches are not triggered when the node is deleted ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-30 12:40 Message: Logged In: YES user_id=12853 Originator: NO I implemented all of your (ben) suggestions except for item 3. This keeps the behavior as close to the original as possible - the default watcher will be called when not sync'd. I register the watcher in finish now - at the very beginning of the function. Please verify this in particular is correct. File Added: watcherobj3.patch ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-30 10:55 Message: Logged In: YES user_id=12853 Originator: NO regarding: 4) as part of reworking the patch I looked closely at the code for triggering watches and based processwatchevent on that, unless I'm missing something what I have is correct behavior -- here is deletenode code on the server: dataWatches.triggerWatch(path, Event.EventNodeDeleted); childWatches.triggerWatch(parentName.equals("")?"/":parentName, Event.EventNodeChildrenChanged); childwatches get childrenchanged on the parent of path while datawatches gets nodedeleted on the path itself 2) so the assumption is that two sets of code won't register the same watch obj? This seems reasonable, but I will make explicit in the docs 3) is that what happens currently? all watches are triggered on disconnect? Otw I'll incorporate the feedback you provided. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-30 09:40 Message: Logged In: YES user_id=154690 Originator: YES This looks much better. There are still a couple of issues: 1) The XXXWatches hashmaps should be final not volatile 2) The elements of the watch hashmaps should be hashsets, not lists. If the same watch is added twice, we only need to invoke once. 3) In processWatchEvent, on disconnect I think all the watches need to be invoked not cleared. This may not really be an issue since I'll be changing this with the auto reregister of watches. 4) In processWatchEvent, the nodeDeleted event gets delivered to both data and child watchers. (If you are watching for children of a node, and that node goes away we trigger a watch.) 5) In processWatchEvent, the remove from the hashmap needs to be done in a synchronized block. Otherwise, you could have concurrent additions to the Hashset, currently ArrayList, going on. 6) You cannot add the watches in the completion function and the blocks of the synchronous calls. If you do you end up with a race condition: you are interested in a change, it comes in right after you get the response but before you actually process it. They all need to be done in one place: finishPacket(); the next packet will not be processed before finishPacket is complete. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-28 15:17 Message: Logged In: YES user_id=12853 Originator: NO I updated the patch based on feedback on original. File Added: watcherobj2.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 13:31 Message: Logged In: YES user_id=154690 Originator: YES As far as the implementation goes, we can't do it in the clientcnxn EventThread since only async events go there. The easiest thing would be to do it in finishPacket(). You should probably add members in Packet to hold the Watcher object put it in watches in finishPacket(). ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 13:26 Message: Logged In: YES user_id=154690 Originator: YES 'Are you saying that if I call getData(node,true) and exists(node,true) and then a subsequent create(node) happens I will get a single watch event of type datachanged for node? I won't see a "datachanged(node)" as well as a "nodecreated(node)", one for each of the read calls respectively?' If a create(node) succeeds, that means the node did not exists, which means that getData got a NoNode error, which means that no watch was set. So, you will only see a nodecreated event and it would be delivered to the Watcher of the exists() call. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:48 Message: Logged In: YES user_id=12853 Originator: NO What would you suggest regarding async read requests and watcher registration - we wrap the callback with code that registers the watcher then calls the client callback? I think this would be cleaner than embedding into the clientcnxn EventThread code. What do you think? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:40 Message: Logged In: YES user_id=12853 Originator: NO Hm. I re-read the docs and I'm still a bit confused by your statements regarding watch types. Are you saying that if I call getData(node,true) and exists(node,true) and then a subsequent create(node) happens I will get a single watch event of type datachanged for node? I won't see a "datachanged(node)" as well as a "nodecreated(node)", one for each of the read calls respectively? The watches wiki http://zookeeper.wiki.sourceforge.net/ZooKeeperWatches should really be beefed up with more detail on which event types are generated based on write ops. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:07 Message: Logged In: YES user_id=12853 Originator: NO I'll rework the patch. Regarding the c vs java - I only implemented java due to the category being set to "java client". Perhaps we should add a "client API" category? Or maybe create two requests instead of 1 -- would be better for me since I'm much more familiar with java than C, you could assign to someone better able to execute. :-) ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 11:22 Message: Logged In: YES user_id=154690 Originator: YES We do also need to implement this in the C client as well, but for simplicity we should make that a separate patch. We shouldn't add watches to the watches hashmap until we get a successful result back from the server. Otherwise, we might trigger watches for things that the client isn't expecting. There is also a problem with type watch types. In the server we have a watch list for data and child watches. It might be good to keep that same division in the client. (Either way would work though.) a watch on getChildren() can result in a childrenChanged or nodeDeleted watch event. getData() can result in dataChanged or nodeDeleted event. exist() can result in dataChanged, nodeCreated, or nodeDeleted event. getChildren() and getData() will not get nodeCreated events, because non existence is an error condition for those two calls and a watch will not be set. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-22 14:09 Message: Logged In: YES user_id=12853 Originator: NO Here's the patch, assigning to Ben for further review. Please pay particular attention to the handling of state changes - I clear the watch table when we are not sync'd to the server. Verify my workflow. File Added: watcherobj.patch ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-20 23:18 Message: Logged In: YES user_id=12853 Originator: NO I've implemented this for Java. I overloaded the respective methods with signatures that take watcher object instead of watch boolean. I added new methods rather than replacing -- this maintains b/w compatibility. 1) should I mark the old methods as deprecated or should we just support both method types (newer signature w/ explicit watcher object && old watch boolean which directs to the "global" watcher.) 2) I see this is listed as category "java client" so I'm assuming there is no need for this feature in c code interface. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-20 17:52 Message: Logged In: YES user_id=12853 Originator: NO I was specifically referring to the client api, not the protocol -- should I assume that that last watcher on a particular node "wins". What I mean is if a client calls "getChildren(path, watcherobj1); getChildren(path, watcherobj2);" (notice same patch) what should the client lib do? I'm assuming use the last watcher, but wanted to verify. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 19:05 Message: Logged In: YES user_id=154690 Originator: YES The client will need to track the multiple watcher objects to invoke when it gets the callback. The client server protocol doesn't change, this is just a client side thing. We should add the ability to remove watches as a separate feature request. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-14 00:21 Message: Logged In: YES user_id=12853 Originator: NO What should we do if the user calls getData multiple times with different watcher objects? Remove previous watch and replace with new watcher, throw error, or allow multiple watchers on particular node? How about removing a watch by calling method on particular node with null watcher? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1937078&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 18:55:21
|
Patches item #1963584, was opened at 2008-05-13 22:56 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: 3.0.0 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Benjamin Reed (breed) Summary: fix build.xml cobertura test harness Initial Comment: Need to fail if the junit tests fail - missing fail is causing build to be successful even if cobertura fails. (ci is running cobertura and not std test target) ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-06 11:55 Message: Logged In: YES user_id=12853 Originator: YES If the tests pass the build will be successful. If a test fails (throws exception or assertX fails) then build will be unsuccessful. This is regardless of println/logging output. Hudson actually parses the output looking for errors (even if build is listed as successful) - if build is successful but there are failure type messages in the stdout/log then it will list the build as unstable, otw will list as success/failure appropriately. I tested the failure case Nigel was seeing and it addressed the issue. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 18:57 Message: Logged In: YES user_id=154690 Originator: NO Is this really enough? It will still show up as unstable build if there are error messages printed even though it returns successfully. Right? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-16 10:44 Message: Logged In: YES user_id=12853 Originator: YES Ben please merge this asap to address ci issue. Thanks. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1963584&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 18:49:49
|
Patches item #1982992, was opened at 2008-06-02 21:38 Message generated for change (Settings changed) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1982992&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Mahadev Konar (mahadevkonar) Assigned to: Benjamin Reed (breed) Summary: concurrency issue in the zookeeper leader code. Initial Comment: The Datatree.ephemerals has a hashset of ephermal nodes for a given session. This list is not synchronized. This was a part of the bug that one of our users found. One of the sessions was getting expired and some other session was deleting ephemeral nodes from the session getting expired. This caused a concurrency modification excpetion while we called hashlist.clone() and datatree.getephermals() --- The fix is simple to put synchronized blocks around the accesses to the list. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-03 11:59 Message: Logged In: YES user_id=154690 Originator: NO Commit to head: Committed revision 171. Commit to tags/2.2.1: Committed revision 173. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-03 11:49 Message: Logged In: YES user_id=154690 Originator: NO +1 thanx mahadev! ---------------------------------------------------------------------- Comment By: Mahadev Konar (mahadevkonar) Date: 2008-06-03 11:35 Message: Logged In: YES user_id=1926680 Originator: YES thanks for catching that Ben :). Attaching a patch that fixes the problem. File Added: concurrency_2.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-03 11:05 Message: Logged In: YES user_id=154690 Originator: NO We need synchronized blocks around all accesses. Since the reads are not in synchronized blocks, you will still get concurrent modification exceptions. ---------------------------------------------------------------------- Comment By: Mahadev Konar (mahadevkonar) Date: 2008-06-02 21:39 Message: Logged In: YES user_id=1926680 Originator: YES ben can you review it? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1982992&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 18:49:25
|
Patches item #1951065, was opened at 2008-04-24 11:43 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1951065&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None >Status: Closed >Resolution: Rejected Priority: 5 Private: No Submitted By: Stefan Groschupf (joa23) Assigned to: Nobody/Anonymous (nobody) Summary: remove system.exist Initial Comment: A patch that replace system exits with Runtime-Exceptions. This improves the capability to use zookeeper embedded in other applications by not crashing the whole application in case ZK crashs. ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-06 11:49 Message: Logged In: YES user_id=154690 Originator: NO Closing, since the patch is not correct. We should probably open a feature request to track this. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-04-25 00:12 Message: Logged In: YES user_id=154690 Originator: NO I think it is doable without much change. We actually shutdown the servers in our test cases. The tricky part is that the shutdown in coming from the inside. Perhaps we just need have an external thread that we can wakeup when we want to shutdown. We need to look at the individual cases to see the best course of action after that thread on woken up so that nothing bad happens before shutdown. In most cases, the error is really severe unexpected cases, so we don't want things to proceed. ---------------------------------------------------------------------- Comment By: Stefan Groschupf (joa23) Date: 2008-04-24 23:24 Message: Logged In: YES user_id=396197 Originator: YES I see. So what would be the best way to solve that? Looks like all other would be some major re-factoring to be able to shutdown all services cleanly. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-04-24 23:16 Message: Logged In: YES user_id=154690 Originator: NO -1 Unfortunately this patch will not do what you want. When a fatal error occurs things need to shutdown, both so that you don't do damage and so that you can restart. If you just throw an exception, there will still be some threads around and some ports open. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1951065&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 18:48:28
|
Bugs item #1984955, was opened at 2008-06-04 17:26 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Benjamin Reed (breed) Summary: Add documentation for Stat class Initial Comment: there seems to be no documentation (api nor wiki) on the stat class. Add documentation detailing the class/fields. Review zookeeper.jute - probably similar issues with other generated classes. Perhaps the docs should be added to zookeeper.jute? Does jute support this? ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-06 11:48 Message: Logged In: YES user_id=154690 Originator: NO Update http://zookeeper.wiki.sourceforge.net/ZooKeeperDataModel with this information. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-06-05 09:21 Message: Logged In: YES user_id=12853 Originator: YES Ben looks like you updated the wiki, I'll let you decide whether this issue has been closed. ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2008-06-05 08:07 Message: Logged In: NO No, Jute doesn't support documentation. It would probably be best to document on the Wiki. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-06 10:03:51
|
Patches item #1986201, was opened at 2008-06-06 11:39 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1986201&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: fpj (fpj) Assigned to: Nobody/Anonymous (nobody) Summary: Vector of Integers (jute) Initial Comment: This patch fixes the problem of creating a vector of integers with jute. Basically, the type of the element of a vector has to be "Integer", and not "int". ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1986201&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-05 18:50:47
|
Bugs item #1985723, was opened at 2008-06-05 11:50 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1985723&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Nobody/Anonymous (nobody) Summary: Improve zk ctor/watcher (state transition) docs Initial Comment: I don't see a succinct doc section on state transition events that are sent to a watcher. (sf search seems to be down so perhaps I missed it). Also need to document the constructor of ZooKeeper class. (missing javadoc) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1985723&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-05 16:21:23
|
Bugs item #1984955, was opened at 2008-06-04 17:26 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) >Assigned to: Benjamin Reed (breed) Summary: Add documentation for Stat class Initial Comment: there seems to be no documentation (api nor wiki) on the stat class. Add documentation detailing the class/fields. Review zookeeper.jute - probably similar issues with other generated classes. Perhaps the docs should be added to zookeeper.jute? Does jute support this? ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-05 09:21 Message: Logged In: YES user_id=12853 Originator: YES Ben looks like you updated the wiki, I'll let you decide whether this issue has been closed. ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2008-06-05 08:07 Message: Logged In: NO No, Jute doesn't support documentation. It would probably be best to document on the Wiki. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-05 15:07:44
|
Bugs item #1984955, was opened at 2008-06-04 17:26 Message generated for change (Comment added) made by nobody You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Nobody/Anonymous (nobody) Summary: Add documentation for Stat class Initial Comment: there seems to be no documentation (api nor wiki) on the stat class. Add documentation detailing the class/fields. Review zookeeper.jute - probably similar issues with other generated classes. Perhaps the docs should be added to zookeeper.jute? Does jute support this? ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2008-06-05 08:07 Message: Logged In: NO No, Jute doesn't support documentation. It would probably be best to document on the Wiki. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-05 00:26:41
|
Bugs item #1984955, was opened at 2008-06-04 17:26 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Patrick Hunt (phunt) Assigned to: Nobody/Anonymous (nobody) Summary: Add documentation for Stat class Initial Comment: there seems to be no documentation (api nor wiki) on the stat class. Add documentation detailing the class/fields. Review zookeeper.jute - probably similar issues with other generated classes. Perhaps the docs should be added to zookeeper.jute? Does jute support this? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008544&aid=1984955&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-03 22:11:25
|
Patches item #1937078, was opened at 2008-04-07 14:21 Message generated for change (Comment added) made by phunt You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1937078&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: java client Group: 3.0.0 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Benjamin Reed (breed) Assigned to: Benjamin Reed (breed) Summary: Passing a watch object to read requests Initial Comment: When using a libraries for higher order synchronization primitives it gets hard to coordinate interests in watch events. It would be much more convenient if a Watcher object could be passed to the getData, getChildren, or exists methods. ---------------------------------------------------------------------- >Comment By: Patrick Hunt (phunt) Date: 2008-06-03 15:11 Message: Logged In: YES user_id=12853 Originator: NO this patch (v4) addresses all of the outstanding issues that were identified and also fixes server bug 1981340. please review and let me know your feedback. Note: I added a bunch of new watcher tests for all of the various sync read operations (exist/getdata/getchildren) for both the legacy and watcher obj registration functions. these tests also verify 1981340. as part of adding the tests I refactored clienttest a bit (extract base). note that I commented out the "sleep(5000)" in the test setup of clientbase. is there a reason why this was there? (works fine for me on all the tests w/o this line). the tests run a whole lot faster if this is commented out - if it needs to be there feel free to add it back. too bad we can't wait() on the the server object... perhaps we should fix this at some point (server does notifyall on state change). File Added: watcherobj4.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-01 08:06 Message: Logged In: YES user_id=154690 Originator: YES Looks good! You are still missing 3 and 4. 3) Since there is currently only one watch, yes all watches are notified of a disconnect. 4) Yes, this is a bug in the server. The current behaviors leave untriggered watches which is not right. I've opened a bug for this: [ 1981340 ] Child watches are not triggered when the node is deleted ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-30 12:40 Message: Logged In: YES user_id=12853 Originator: NO I implemented all of your (ben) suggestions except for item 3. This keeps the behavior as close to the original as possible - the default watcher will be called when not sync'd. I register the watcher in finish now - at the very beginning of the function. Please verify this in particular is correct. File Added: watcherobj3.patch ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-30 10:55 Message: Logged In: YES user_id=12853 Originator: NO regarding: 4) as part of reworking the patch I looked closely at the code for triggering watches and based processwatchevent on that, unless I'm missing something what I have is correct behavior -- here is deletenode code on the server: dataWatches.triggerWatch(path, Event.EventNodeDeleted); childWatches.triggerWatch(parentName.equals("")?"/":parentName, Event.EventNodeChildrenChanged); childwatches get childrenchanged on the parent of path while datawatches gets nodedeleted on the path itself 2) so the assumption is that two sets of code won't register the same watch obj? This seems reasonable, but I will make explicit in the docs 3) is that what happens currently? all watches are triggered on disconnect? Otw I'll incorporate the feedback you provided. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-30 09:40 Message: Logged In: YES user_id=154690 Originator: YES This looks much better. There are still a couple of issues: 1) The XXXWatches hashmaps should be final not volatile 2) The elements of the watch hashmaps should be hashsets, not lists. If the same watch is added twice, we only need to invoke once. 3) In processWatchEvent, on disconnect I think all the watches need to be invoked not cleared. This may not really be an issue since I'll be changing this with the auto reregister of watches. 4) In processWatchEvent, the nodeDeleted event gets delivered to both data and child watchers. (If you are watching for children of a node, and that node goes away we trigger a watch.) 5) In processWatchEvent, the remove from the hashmap needs to be done in a synchronized block. Otherwise, you could have concurrent additions to the Hashset, currently ArrayList, going on. 6) You cannot add the watches in the completion function and the blocks of the synchronous calls. If you do you end up with a race condition: you are interested in a change, it comes in right after you get the response but before you actually process it. They all need to be done in one place: finishPacket(); the next packet will not be processed before finishPacket is complete. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-28 15:17 Message: Logged In: YES user_id=12853 Originator: NO I updated the patch based on feedback on original. File Added: watcherobj2.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 13:31 Message: Logged In: YES user_id=154690 Originator: YES As far as the implementation goes, we can't do it in the clientcnxn EventThread since only async events go there. The easiest thing would be to do it in finishPacket(). You should probably add members in Packet to hold the Watcher object put it in watches in finishPacket(). ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 13:26 Message: Logged In: YES user_id=154690 Originator: YES 'Are you saying that if I call getData(node,true) and exists(node,true) and then a subsequent create(node) happens I will get a single watch event of type datachanged for node? I won't see a "datachanged(node)" as well as a "nodecreated(node)", one for each of the read calls respectively?' If a create(node) succeeds, that means the node did not exists, which means that getData got a NoNode error, which means that no watch was set. So, you will only see a nodecreated event and it would be delivered to the Watcher of the exists() call. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:48 Message: Logged In: YES user_id=12853 Originator: NO What would you suggest regarding async read requests and watcher registration - we wrap the callback with code that registers the watcher then calls the client callback? I think this would be cleaner than embedding into the clientcnxn EventThread code. What do you think? ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:40 Message: Logged In: YES user_id=12853 Originator: NO Hm. I re-read the docs and I'm still a bit confused by your statements regarding watch types. Are you saying that if I call getData(node,true) and exists(node,true) and then a subsequent create(node) happens I will get a single watch event of type datachanged for node? I won't see a "datachanged(node)" as well as a "nodecreated(node)", one for each of the read calls respectively? The watches wiki http://zookeeper.wiki.sourceforge.net/ZooKeeperWatches should really be beefed up with more detail on which event types are generated based on write ops. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-27 12:07 Message: Logged In: YES user_id=12853 Originator: NO I'll rework the patch. Regarding the c vs java - I only implemented java due to the category being set to "java client". Perhaps we should add a "client API" category? Or maybe create two requests instead of 1 -- would be better for me since I'm much more familiar with java than C, you could assign to someone better able to execute. :-) ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-27 11:22 Message: Logged In: YES user_id=154690 Originator: YES We do also need to implement this in the C client as well, but for simplicity we should make that a separate patch. We shouldn't add watches to the watches hashmap until we get a successful result back from the server. Otherwise, we might trigger watches for things that the client isn't expecting. There is also a problem with type watch types. In the server we have a watch list for data and child watches. It might be good to keep that same division in the client. (Either way would work though.) a watch on getChildren() can result in a childrenChanged or nodeDeleted watch event. getData() can result in dataChanged or nodeDeleted event. exist() can result in dataChanged, nodeCreated, or nodeDeleted event. getChildren() and getData() will not get nodeCreated events, because non existence is an error condition for those two calls and a watch will not be set. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-22 14:09 Message: Logged In: YES user_id=12853 Originator: NO Here's the patch, assigning to Ben for further review. Please pay particular attention to the handling of state changes - I clear the watch table when we are not sync'd to the server. Verify my workflow. File Added: watcherobj.patch ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-20 23:18 Message: Logged In: YES user_id=12853 Originator: NO I've implemented this for Java. I overloaded the respective methods with signatures that take watcher object instead of watch boolean. I added new methods rather than replacing -- this maintains b/w compatibility. 1) should I mark the old methods as deprecated or should we just support both method types (newer signature w/ explicit watcher object && old watch boolean which directs to the "global" watcher.) 2) I see this is listed as category "java client" so I'm assuming there is no need for this feature in c code interface. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-20 17:52 Message: Logged In: YES user_id=12853 Originator: NO I was specifically referring to the client api, not the protocol -- should I assume that that last watcher on a particular node "wins". What I mean is if a client calls "getChildren(path, watcherobj1); getChildren(path, watcherobj2);" (notice same patch) what should the client lib do? I'm assuming use the last watcher, but wanted to verify. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-05-19 19:05 Message: Logged In: YES user_id=154690 Originator: YES The client will need to track the multiple watcher objects to invoke when it gets the callback. The client server protocol doesn't change, this is just a client side thing. We should add the ability to remove watches as a separate feature request. ---------------------------------------------------------------------- Comment By: Patrick Hunt (phunt) Date: 2008-05-14 00:21 Message: Logged In: YES user_id=12853 Originator: NO What should we do if the user calls getData multiple times with different watcher objects? Remove previous watch and replace with new watcher, throw error, or allow multiple watchers on particular node? How about removing a watch by calling method on particular node with null watcher? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1937078&group_id=209147 |
|
From: SourceForge.net <no...@so...> - 2008-06-03 18:59:38
|
Patches item #1982992, was opened at 2008-06-02 21:38 Message generated for change (Comment added) made by breed You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1982992&group_id=209147 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: server Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Mahadev Konar (mahadevkonar) Assigned to: Benjamin Reed (breed) Summary: concurrency issue in the zookeeper leader code. Initial Comment: The Datatree.ephemerals has a hashset of ephermal nodes for a given session. This list is not synchronized. This was a part of the bug that one of our users found. One of the sessions was getting expired and some other session was deleting ephemeral nodes from the session getting expired. This caused a concurrency modification excpetion while we called hashlist.clone() and datatree.getephermals() --- The fix is simple to put synchronized blocks around the accesses to the list. ---------------------------------------------------------------------- >Comment By: Benjamin Reed (breed) Date: 2008-06-03 11:59 Message: Logged In: YES user_id=154690 Originator: NO Commit to head: Committed revision 171. Commit to tags/2.2.1: Committed revision 173. ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-03 11:49 Message: Logged In: YES user_id=154690 Originator: NO +1 thanx mahadev! ---------------------------------------------------------------------- Comment By: Mahadev Konar (mahadevkonar) Date: 2008-06-03 11:35 Message: Logged In: YES user_id=1926680 Originator: YES thanks for catching that Ben :). Attaching a patch that fixes the problem. File Added: concurrency_2.patch ---------------------------------------------------------------------- Comment By: Benjamin Reed (breed) Date: 2008-06-03 11:05 Message: Logged In: YES user_id=154690 Originator: NO We need synchronized blocks around all accesses. Since the reads are not in synchronized blocks, you will still get concurrent modification exceptions. ---------------------------------------------------------------------- Comment By: Mahadev Konar (mahadevkonar) Date: 2008-06-02 21:39 Message: Logged In: YES user_id=1926680 Originator: YES ben can you review it? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1008546&aid=1982992&group_id=209147 |
