Session expired bad behavior

[vvo]

Hello.

Since you switched to your mcavage/node-zookeeper fork, there's no more a 'close' event sent when a zookeeper session expires.

We have been able to reproduce this, very easy:

• Start a zookeeper server in a VirtualBox
• Connect to your zookeeper server from your host machine, with a very low timeout (seems 4000ms is the lowest), using latest node-zkplus
• Stop communication between zookeeper server and your client: sudo ifconfig vboxnet0 down
• Wait 5-6s
• Restart communication: sudo ifconfig vboxnet0 up

What happens:

• all watches are getting -1 etc.. as seen in https://github.com/yfinkelstein/node-zookeeper#known-bugs--issues

Expected behavior:

• a close event should be sent by the native zookeeper lib

Your fork does not send a close event, thus our code around session disconnection and reconnect does not work anymore: 6a97b30

Putting back "official" node-zookeeper works well, because we do receive a close event when session expires.

I guess we could hack

node-zkplus/lib/client.js

Line 799 in 733906c

emitter.emit(event);

to watch for session event and call onClose manually.

What do you think ?

PS1:
I hope you do monitor your zk connection status from a client side.
Because when session expires (eg: network interruption, garbage collector working for more than zookeeper timeout), you will never know it ONLY if you do watch for -1 state in your watches.

PS2:
We could not understand how is it possible that our nodejs client and zookeeper server were not able to reach each other for 30 seconds (our timeout), did you already have experienced similar issues with node-zookeeper? (your fork, the official)

[mcavage]

Hi Vincent,

Thanks for the report. Well, I really want to have node-zookeeper merge in
my changes, but until they do zkplus can't use it in 0.8, so I guess the
best thing would be either to hack around it in zkplus, or actually just
fix this in the fork, since it's already diverged. I've talked with Yuri
about merging mine back in, and I know he wants to, I'm guessing this just
isn't high on his priority list - long winded way of saying "if we fix it
in the fork it will probably make its way back to mainline eventually".

I probably don't see this because I do that poll on getState bit (i.e.,
"yes, I am aggressively monitoring the client state").

Thoughts?

m

On Tue, Sep 25, 2012 at 8:39 AM, Vincent Voyer notifications@github.comwrote:

Hello.

Since you switched to your mcavage/node-zookeeper fork, there's no more an
'close' event sent when a zookeeper session expires.

We have been able to reproduce this, very easy:

• Start a zookeeper server in a VirtualBox
• Connect to your zookeeper server from your host machine, with a very
  low timeout (seems 4000ms is the lowest), using latest node-zkplus
• Stop communication between zookeeper server and your client: sudo
  ifconfig vboxnet0 down
• Wait 5-6s
• Restart communication: sudo ifconfig vboxnet0 up

What happens:

• all watches are getting -1 etc.. as seen in
  https://github.com/yfinkelstein/node-zookeeper#known-bugs--issues

Expected behavior:

• a close event been sent by the native zookeeper lib

Your fork does not send a close event, thus our code around session
disconnection and reconnect does not work anymore: 6a97b306a97b30

Putting back "official" node-zookeeper works well, because we do receive a
close event when session expires.

I guess we could hack
https://github.com/mcavage/node-zkplus/blob/733906cd7f7faea642cfb81c1db8c5c7f7f1f056/lib/client.js#L799to watch for session event and call onClose manually.

What do you think ?

Also, I hope you do monitor your zk connection status from a client side.

Because when session expires (eg: network interruption, garbage collector
working for more than zookeeper timeout), you will never know it ONLY if
you do watch for -1 state in your watches.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/15.

[vvo]

I switched back to node-zookeeper in our zkplus fork and it seems to work well.
I know you already told me not to do so but for now it works.

About polling the state.
I feel that if I need to do a setInterval() to check zookeeper state and deal with it, then it means I do not need zookeeper at all and better do my own simple datastore and poll it with setInterval()!

This compare to websockets() and long polling in browser. If I have websockets and I still need to setInterval an ajax request then I failed.

Perhaps I am wrong on this as I do not have a long experience with zookeeper and do not understand the whole idea behind it.