[CURATOR-349] Trace the session id even when it is disconnected #172
Conversation
| } | ||
| } catch (Exception e) { | ||
| // Ignore the exception | ||
| long sessionId = 0; |
There was a problem hiding this comment.
Why did we lose the isConnected() check?
There was a problem hiding this comment.
We also want to track the sessionId when the connection timed-out happened, also it would be useful if we can know what's the disconnected session id when the create/setData/getData is called.
Any concern of removing the isConnected() check?
There was a problem hiding this comment.
My concern is that zooKeeper.getZooKeeper() can cause a new connection to get started. I'm not sure that that kind of side-effect is expected. If there is no current connection or the connection is in error somehow, zooKeeper.getZooKeeper() will call HandleHolder.closeAndReset().
There was a problem hiding this comment.
@Randgalt, here we're calling the HandleHolder.getZookeeper() directly, so it won't call closeAndReset(), please correct me if I missed anything.
There was a problem hiding this comment.
Yes, I think you're right. I think I can accept this.
lvfangmin
changed the title
|
(Not related with this diff) The ZOOKEEPER-1525 looks good to me except the new additional parameters concern, if we can refactor that a bit, I'll add a +1 and ask Ben (one of the ZooKeeper committer, also my team member) to accelerate the review process and accept it. |
|
Btw, I also like to get this in the next release :) |
|
I'll start working on a release |
|
Thanks @Randgalt, please let me know when the new release is published 👍 |
|
Hi @Randgalt, hope you have a great weekend :) |
|
@lvfangmin do you subscribe to dev@curator? We released this morning and an email was sent to that list. So, the bits are available now. |
|
Ah, just saw it, thanks @Randgalt! |
The default session id inside ZooKeeper is 0, we should keep consistent with it in tracker.