> On May 24, 2013, 4:31 p.m., Raul Gutierrez Segales wrote: > > src/detector/detector.cpp, line 254 > > <https://reviews.apache.org/r/11366/diff/2/?file=296366#file296366line254> > > > > I think this belongs inside zookeeper.cpp:ZooKeeper#create so that we > > guard ourselves for all callsites. > > Vinod Kone wrote: > Hmm. I'm not convinced thats the right strategy. We want to expose basic > primitives (e.g. exists(), create()) and let the callers use composition > (e.g., here if not exists() then create()) to do what they want to do. I > think this gives us the greatest flexibility in using zookeeper.hpp. > > Raul Gutierrez Segales wrote: > Well, you are creating paths recursively.. that's not basic so it could > have some more logic. But I am new to this code base so that was just an > opinion. > > And also, with the current approach we would still be issuing a lot of > unneeded create calls (i.e.: if you call it for /something/very/nested then > you would still have a bunch of unneeded calls). > > What about extending create() to have a new param (i.e.: check_exists) > that adds this new behavior? And we could add a comment that it's the > recommended way to reduce load on ZK since create() ops are quorum ops.
Ended up modifying the recursive create to check for existence of each intermediate node. It turned out to be a little complicated. Take a look. I still kept the exists() check in detector.cpp, because I think it reduces the round trip time compared to just relying on create(). - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11366/#review21004 ----------------------------------------------------------- On May 24, 2013, 4:23 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11366/ > ----------------------------------------------------------- > > (Updated May 24, 2013, 4:23 p.m.) > > > Review request for mesos, Benjamin Hindman, Bill Farner, Ben Mahler, and Raul > Gutierrez Segales. > > > Description > ------- > > This is likely a short term fix pending Yan's refactor of detector. > > 2 changes: > 1) Fixed a bug in zookeeper->create(). > 2) Removed create for non-contending detectors. > > > This addresses bug mesos-409. > https://issues.apache.org/jira/browse/mesos-409 > > > Diffs > ----- > > src/detector/detector.cpp 12deefa0b9df3f4946d80f500caaa5199b8ea28e > src/zookeeper/zookeeper.hpp 99e689e5178845480b2426e694d18c5257234166 > src/zookeeper/zookeeper.cpp 267c38a2922f114519ffaf4f0bdce74d22fc1506 > > Diff: https://reviews.apache.org/r/11366/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
