-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow reconnection to a session from a saved session id and passwd #63
base: master
Are you sure you want to change the base?
Conversation
2f47f5c
to
7f25d2f
Compare
conn.go
Outdated
@@ -333,6 +341,15 @@ func (c *Conn) SessionID() int64 { | |||
return atomic.LoadInt64(&c.sessionID) | |||
} | |||
|
|||
// Password returns the current session password of the connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a bit overkill. Why not just at a RWMutex to Conn?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it's simpler, this was to match the way sessionID
is accessed. I've changed to use a RWMutex.
Codecov Report
@@ Coverage Diff @@
## master #63 +/- ##
==========================================
+ Coverage 75.27% 75.79% +0.52%
==========================================
Files 7 7
Lines 1189 1194 +5
==========================================
+ Hits 895 905 +10
+ Misses 202 198 -4
+ Partials 92 91 -1
Continue to review full report at Codecov.
|
91f92cb
to
b382a90
Compare
Hi @nemith, |
@nemith is there something blocking this change from being reviewed? |
This PR adds a way to read the session id and session password from an established session and to provide them as argument when creating a new connection, allowing to recover an existing session. (This mimicks what the java Zookeeper allows: https://zookeeper.apache.org/doc/r3.3.3/api/org/apache/zookeeper/ZooKeeper.html)
A possible use case for this is to store these values in persistent storage and re-establish a session after a process restart.
Operations to get and set
passwd
are implemented with atomic operations on an unsafe pointer to be consistent with the atomic operations used forsessionID
but I'm happy to change the PR to use a lock instead if that's preferable.I have also made a small change to the
waitForSession
helper that I think is consistent with what the function tries to achieve.Locally all the tests are passing except for
TestSetWatchers
but it also fails on master with the following error: