rust-zookeeper
rust-zookeeper copied to clipboard
bonifaido
Implement classic-ZK/Curator recipes
- [ ] Lock (in progress)
- [ ] LeaderSelector (depends on Lock I guess)
- [x] PathChildrenCache (migrate to Curator)
- [ ] ServiceDiscovery (depends on PathChildrenCache)
i'd really like to have service discovery added to the list. was playing around with pathchildrencache today (i see you did too) since that would be useful stepping stone.
Great idea, I have updated the list. Right now I'm experimenting with PathChildrenCache, how it would should be done. I have seen you have some changes around there already. Let me know if you have something working.
i've focussed on copying the way that the java implementation serialises its operations using a worker thread. it seems like it could work. i've not had to change your get_data and get_children, except to patch one place in get_children where it computes the path of the child. I get //test if i use / as the parent path, which isn't valid.
i am confused about rust's usage of traits with closures -- everywhere takes a Fn watcher closure, except the Zookeeper::connect method which takes a Watcher trait. they don't seem to be completely equivalent. i couldn't pass in the LoggingWatcher to get_children_w for example.
Great, I'm happy that you took the chance to make this recipe better.
Traits/closureres: I'm not sure if this is the right way to go, but my intention with FnOnce was that if you pass in a closure as FnOnce the call takes it by value, so you can pass towards anything captured and accumulated in the closure to a next call (re-subscribe). I think after your changes in PathChildrenCache this may become unnecessary. If you think this is inconvenient or looks strange I'm ok with changing it!
i've had some success getting this to work. still to do is to feed the resulting node events out to users of the cache. and some more testing.
i had some trouble getting out of the connecting state though. https://github.com/cmsd2/rust-zookeeper/commit/50d824af0bce13183c95c85d1d1cefeae3eaa0ca seemed to help, but i'm quite uncertain it's the right fix.
i've updated my branch with something that works end-to-end. comments welcome.
tried it on a real zookeeper instance:
watch children -> Ok(["test", "hbase", "sandbox-discovery", "zookeeper", "distributed-config", "sandbox", "consumers", "kafka", "discovery", "data-versions", "service-paths"]) cache started press enter to close client Illegal instruction (core dumped)
that was using rustc 1.4.0-nightly (7bf626a68 2015-09-07)
I don't think I've got the time or ability to fix that myself
Interesting, I will have a look at this at weekend. Could you please check how much bytes do these child nodes contain?
well /test was the ephemeral node created by the example itself. the rest except for /service-paths are actually just used as folders for more stuff and so all have 0 bytes of data attached to them. /service-paths is 71102 bytes. it tries to get the nodes exactly in the order as printed in that list, and it bombs out during trying to get data for /consumers, but there's nothing special about that one.
better log of the trace output: https://gist.github.com/cmsd2/bb87b48dc5158e09a5d9
I have found a potential issue which blow up with almost the same stack trace as for you, I have fixed it in https://github.com/bonifaido/rust-zookeeper/commit/653ffed4f96dd32d051ed0636df10837e539b5fa. However I have re-produced this a with node with data size near to the limit (1MB). Could you please check if this has resolved your issue?
If not, could you please tell, the ZK version, rust version, and which program are you running, I guess the example. Also do you get this error repeatedly or it happened just once?
it's quite repeatable, no your change doesn't help in my case. yes, the zookeeper_example as per my master branch.
i've just noticed the large value that is passed to vec::from_elem comes from proto.rs where it tries a read_i32 as usize in read_buffer. the value i can see in the stack trace is actually -1 coerced to an unsigned 64bit number.
a fix that does work for me is to clamp the value to be non-negative so in the case where i get -1 i squash it to 0 and end up returning a 0-length vector. i don't know what the underlying protocol or lower level code is doing exactly, but if -1 means end-of-message then that would explain why it's ok to return a 0-length vector
https://github.com/cmsd2/rust-zookeeper/commit/3db01840c22f82bf1856cb3e00a8fa4f5cfcadc5
zookeeper version is 3.4.5 from cloudera's CDH 4.7.1
still to do: check corner cases: what happens if things change while not connected?
Ok, I'm trying to reproduce the exact same environment, I'm using your fork of the library and running the example program compiled with current rust nightly (I don't have the -1/0 fix that you just added). I have also downgraded to ZK 3.4.5 (I was using 3.4.6 until now).
I have the following nodes under '/' : consumers data-versions discovery distributed-config hbase kafka sandbox sandbox-discovery service-paths (71102 bytes, others are 0 bytes) zookeeper
However still no luck with reproducing the issue, do I miss something?
However you are right, -1 can be a valid length in responses according to the original implementation https://github.com/apache/zookeeper/blob/9ab53eac45cfee91cbceee1938962a85d411bd84/src/java/main/org/apache/jute/BinaryInputArchive.java#L91 I'm digging what does that mean, maybe simply just a representation of no data, anyhow, it is a valid fix, so I'm happy to merge your changes :)
i seem to be able to control whether the server stores a null byte array for the data or a 0-length array by sending either -1 or 0 from the client. you can try it by modifying proto.rs to replace try!(self.data.write_to(writer)) with try!(writer.write_i32::<BigEndian>(-1)) in CreateRequest::write_to.
the example doesn't use get_data immediately after create (it does a set_data first), so it won't fail until you modify it. if you do add in the extra get_data then fail it does.
Fair enough, ok, so let's leave the len check with -1 there in read_buffer() because it seems to be a completely valid scenario in the original zk client as well.
ok i think our cache impl is now in a useable state.
but there are some differences between it and the java version: we have fewer options for the initialisation mode, we don't de-dup in-flight operations as they're marshalled onto the worker thread there's no option to cache just nodes, not data i'm not certain i've got the error handling right
The changes on your branch look good to me, I think it is in a status so that we can merge it into the repo. The whole project is in WIP status anyhow.
Operation doesn't need to be pub I think.
If you think you are done with the initial implementation please create a pull request, we can always fix and discuss fixes and other additions later on. And thanks for the effort so far!
I'm working on a Mutex impl https://github.com/cmsd2/rust-zookeeper/tree/mutex
i really want std::io::Timer but it got removed in favour of sleep_ms for now. I've had to spawn a timer thread as needed, which isn't great. I think the design is sound, but of course the more people check it for correctness the better. it could do with some kind of backoff to prevent dogpiling when the lock becomes available.
oh and i would have liked Thread to implement PartialEq. instead I've had to define a ThreadId type which is platform specific (pthread_self or equiv).
and the mutex isn't a rust style mutex; there's no guard object at the moment.
For the Timer problem for some time schedule_recv was part of the library, but I have removed it because it became unnecessary: https://github.com/PeterReid/schedule_recv It uses only one static thread, feel free to use it, I haven't had any problems with it (it was sending the pings). It got removed when I changed to whole thing to mio, maybe the mio timeout facility can solve your issue, I haven't checked it.
thanks that works great. i will aim for compatability with curator, but i haven't quite got there yet.
Does it make sense to have curator as a sibling project to a more lightweight zookeeper?
Yes, absolutely, some people wouldn't need the whole stuff with curator, for example in cases like reading/writing simple a key from/to zookeeper.