reconnect if failed #58
Conversation
|
I am not sure this will do what you want it to do. connect() already has a retry loop inside of it and it only returns errors if c.shoudlExit is true. What issues are you running into that you think this change is needed? What testing have you done before/after the change? Can you show some test results? Unit tests would be good as well, but like i stated above I am not sure this will do what we want to to do, however perhaps there is another issue you are trying to solve? |
| @@ -423,7 +423,9 @@ func (c *Conn) loop(ctx context.Context) { | |||
| for { | |||
| if err := c.connect(); err != nil { | |||
| // c.Close() was called | |||
| return | |||
| c.logger.Printf("connect failed: %s", err) | |||
| time.Sleep(time.Second) | |||
| @@ -423,7 +423,9 @@ func (c *Conn) loop(ctx context.Context) { | |||
| for { | |||
| if err := c.connect(); err != nil { | |||
There was a problem hiding this comment.
connect seems to only return an error when c.shouldQuit is set. connect already has a reconnect loop as well, so I am not sure this will do what you want it to do.
There was a problem hiding this comment.
case: zk server startup AFTER zk client
There was a problem hiding this comment.
connect() failed and then RETURN loop()
There was a problem hiding this comment.
Could you help explain this a bit more for me. I am sure you are right, but I am not picturing the scenario.
There was a problem hiding this comment.
Also any unittest here would be good. The logic here is not trivial and a unittest could document the issue as well protect against regressions
Codecov Report
@@ Coverage Diff @@
## master #58 +/- ##
==========================================
+ Coverage 75.77% 76.65% +0.88%
==========================================
Files 7 7
Lines 1189 1191 +2
==========================================
+ Hits 901 913 +12
+ Misses 197 190 -7
+ Partials 91 88 -3
Continue to review full report at Codecov.
|
reconnect if failed