Support retry logic for auto recovery#3799
Conversation
Signed-off-by: xiaolongran <xiaolongran@tencent.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3799 +/- ##
============================================
+ Coverage 60.47% 60.51% +0.03%
- Complexity 5847 5850 +3
============================================
Files 473 473
Lines 40933 40937 +4
Branches 5235 5237 +2
============================================
+ Hits 24756 24772 +16
+ Misses 13976 13951 -25
- Partials 2201 2214 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } catch (Exception e) { | ||
| LOG.error("Exception while performing auditor election", e); | ||
| submitShutdownTask(); | ||
| if (e.getCause() instanceof KeeperException.ConnectionLossException |
There was a problem hiding this comment.
It's better to retry with a delay time I think.
There was a problem hiding this comment.
IIRC ConnectionLoss is a non-recoverable exception.
To handle it properly, one has to re-create ZK client and deal with the watchers/cached state. Ephemeral nodes expire as result of such error too.
Re-initializing the client at random place when error happens is a rather big chunk of work IIRC and it easier to bounce the service in this case, hence #3374
There was a problem hiding this comment.
@dlg99 I saw it said connection loss is a recoverable exception.
Recoverable errors: the disconnected event, connection timed out, and the connection loss exception are examples of recoverable errors
The session expiration is a fatal error. We should re-create ZK client
dlg99
left a comment
dlg99
left a comment
There was a problem hiding this comment.
- Please see #3374 and https://cwiki.apache.org/confluence/display/ZOOKEEPER/ErrorHandling
- IIRC ConnectionLoss (or SessionExpiredEx or both) is a non-recoverable exception. To handle it properly, one has to re-create ZK client and deal with the watchers/cached state. Ephemeral nodes expire as result of such error too. Re-initializing the client at random place when error happens is a rather big chunk of work IIRC and it easier to bounce the service in this case, hence #3374
- Change without unit tests/repro
If you disagree with my assessment please start a discussion in the mailing list.
@eolivelli has deeper knowledge of zookeeper, let's double-check with him too.
It is not a good idea in general to run the autorecovery as part of the bookie in prod. The best practice is to run it as a separate service on either another host (which allows one to have fewer AR service nodes than bookies) or alongside with the bookie as a separate process. Running AR separately also helps with such issues like AR activity resulting in a more frequent java GC cycles. HTH.
| } catch (Exception e) { | ||
| LOG.error("Exception while performing auditor election", e); | ||
| submitShutdownTask(); | ||
| if (e.getCause() instanceof KeeperException.ConnectionLossException |
There was a problem hiding this comment.
IIRC ConnectionLoss is a non-recoverable exception.
To handle it properly, one has to re-create ZK client and deal with the watchers/cached state. Ephemeral nodes expire as result of such error too.
Re-initializing the client at random place when error happens is a rather big chunk of work IIRC and it easier to bounce the service in this case, hence #3374
5 participants
Descriptions of the changes in this PR:
Motivation
In the current implementation, when the Session between Bookie and ZK expires, Auto Recovery does not support automatic reconnection. However, the bookie server itself supports reconnection logic, so when Auto Recovery and Bookie Server are deployed together, once the Session expires with ZK, Auto Recovery will automatically stop and cause the entire Bookie service to go down.
However, in a production environment, session expiration between Bookie and ZK itself is a high-frequency operation, such as the following scenario:
From the ZK side, we can see that the current ZK zxid has overflowed, triggering XidRolloverException:
When the above situation occurs, we can see that the Session between Bookie and ZK expires:
Since Auto Recovery has no reconnection logic, the entire Bookie service is down.
Changes