That's a point. TBH I don't remember the rationale for how it is today. I'm loathe to change working code though. Are you seeing a problem?
-Jordan > On Jun 7, 2017, at 7:15 PM, Jeff Clites <[email protected]> wrote: > > At the bottom of the LeaderLatch.checkLeadership() method (in Curator > 2.12.0), the following is called to set a watch on the waiter one spot ahead > of us, as expected: > > String watchPath = sortedChildren.get(ourIndex - 1); > ... > client.getData().usingWatcher(watcher).inBackground(callback).forPath(ZKPaths.makePath(latchPath, > watchPath)); > > When the watch fires and has event type NodeDeleted, then > LeaderLatch.getChildren() is called to determine if we are now the leader, > again as expected. > > But I don't understand the following. The code for the callback (called in > response to the getData() completing) has this: > > if ( event.getResultCode() == KeeperException.Code.NONODE.intValue() ) > { > // previous node is gone - reset > reset(); > } > > It seems that this should be a call to getChildren(), rather than to reset(), > since this means that the waiter ahead of us has disappeared (the same > situation we were watch-ing for, but it happened before we could even set a > watch), whereas reset() would be the right reaction if our path had > disappeared. It looks to me like this code "thinks" it's a watch about our > node, rather than the one-ahead-of-use node. I think this would cause us to > delete and re-create our node, effectively moving us to the back of the wait > queue, rather than making us the leader now. > > Is this a bug, or am I misinterpreting something? > > Looking at the history of this code, it looks like it did in fact used to > check for leadership at this point, and got changed when the code was updated > to use background calls (in commit sha ff4ec29f). > > Thanks in advance, > > Jeff Clites
