Fix for stale session issue #37
Conversation
Codecov Report
@@ Coverage Diff @@
## master #37 +/- ##
==========================================
- Coverage 77.58% 76.73% -0.86%
==========================================
Files 7 7
Lines 1316 1199 -117
==========================================
- Hits 1021 920 -101
+ Misses 205 190 -15
+ Partials 90 89 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hi, @colinmcintosh, thanks for your submission. Do you think you could provide a test which exercises those code paths?
|
I am trying to understand what is this change fixing. Doesn't the session get reset in here for you https://github.com/go-zookeeper/zk/blob/master/conn.go#L673? |
|
@pmazzini |
|
Any way we can get a unit test on this? |
|
If this can help someone, I got this |
nemith
added
needs investigatation
colinmcintosh
force-pushed
the
fix/authentication
branch
from
51e0514 to
0607fee
Compare
|
Hi! I pushed a couple updates to restrict this session reset further and moved the disconnect time to fix an issue with tight loops not being caught by the timer. I did attempt to write a test for this that started, stopped, and started the test ZK server to reset the session but couldn't figure out how to make it work with the test server. I'm able to easily reproduce this by running Zookeeper in a Docker container and restarting the container ( |
colinmcintosh
force-pushed
the
fix/authentication
branch
from
0607fee to
a4c549f
Compare
…nd does not reconnect before the session timeout time elapses.
colinmcintosh
force-pushed
the
fix/authentication
branch
from
a4c549f to
127b9eb
Compare
|
I will take a stab at a unit test to produce the issue when I find some magic 🦄 time. 🤣 No promises. |
| @@ -482,6 +490,7 @@ func (c *Conn) loop(ctx context.Context) { | |||
| } | |||
|
|
|||
| c.setState(StateDisconnected) | |||
| disconnectTime = time.Now() | |||
There was a problem hiding this comment.
i think this should place after line 489;
when zookeeper cluster re-elect, reconnect always success, but auth failed:(EOF). line 493 will rfresh disconnectTime every loop; line 442 always be False
|
Hi @colinmcintosh, are you still working on this pull request. I'm facing similar issue and hoping this pull request will be done. |
|
Hi, I am still experiencing this issue. |
Added logic to reset the client SessionID if the client disconnects and does not reconnect before the session timeout time elapses.
make testpasses for me with these changes.Fixes #36.