go-zookeeper / zk

Native ZooKeeper client for Go
BSD 3-Clause "New" or "Revised" License
512 stars 130 forks source link

Allow reconnection to a session from a saved session id and passwd #63

Open bmermet opened 3 years ago

bmermet commented 3 years ago

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 for sessionID 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:

2021/08/12 16:38:09 connected to 127.0.0.1:14108
2021/08/12 16:38:09 connected to 127.0.0.1:14108
2021/08/12 16:38:09 authenticated: id=72088870267584512, timeout=10000
2021/08/12 16:38:09 authenticated: id=72088870267584513, timeout=10000
2021/08/12 16:38:09 re-submitting `0` credentials after reconnect
2021/08/12 16:38:09 re-submitting `0` credentials after reconnect
2021/08/12 16:38:29 recv loop terminated: failed to read from connection: read tcp 127.0.0.1:62986->127.0.0.1:14108: use of closed network connection
2021/08/12 16:38:29 send loop terminated: <nil>
2021/08/12 16:38:50 connected to 127.0.0.1:14108
2021/08/12 16:38:50 authentication failed: zk: session has been expired by the server
2021/08/12 16:38:51 connected to 127.0.0.1:14108
2021/08/12 16:38:51 authenticated: id=72088870267584514, timeout=10000
2021/08/12 16:38:51 re-submitting `0` credentials after reconnect
2021/08/12 16:39:10 recv loop terminated: EOF
2021/08/12 16:39:10 send loop terminated: <nil>
2021/08/12 16:39:10 recv loop terminated: EOF
2021/08/12 16:39:10 send loop terminated: <nil>
--- FAIL: TestSetWatchers (63.04s)
    cluster_test.go:15: [ZKERR] ZooKeeper JMX enabled by default
    cluster_test.go:15: [ZKERR] Using config: /var/folders/79/rrhrlc6d5f1gd1dqxvnl468h0000gn/T/gozk873519656/srv1/zoo.cfg
    zk_test.go:828: GetW watcher error zk: session has been expired by the server
FAIL
exit status 1
FAIL    github.com/go-zookeeper/zk  63.484s
codecov[bot] commented 3 years ago

Codecov Report

Merging #63 (7f25d2f) into master (8866d76) will increase coverage by 0.52%. The diff coverage is 100.00%.

:exclamation: Current head 7f25d2f differs from pull request most recent head 083a313. Consider uploading reports for the commit 083a313 to get more accurate results Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
conn.go 72.58% <100.00%> (+1.00%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8866d76...083a313. Read the comment docs.

bmermet commented 3 years ago

Hi @nemith, Would you be able to have another look at this PR? Let me know if there are other things I can improve.

bmermet commented 2 years ago

@nemith is there something blocking this change from being reviewed? Any guidance on how to get this PR moving forward would be much appreciated.