sijie commented on a change in pull request #1747: Set ConnectionExpired
Listener to MetadataClientDriver in AR
URL: https://github.com/apache/bookkeeper/pull/1747#discussion_r224547592
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AutoRecoveryMain.java
##########
@@ -91,6 +92,11 @@ public AutoRecoveryMain(ServerConfiguration conf,
StatsLogger statsLogger)
CompatibilityException {
this.conf = conf;
this.bkc = Auditor.createBookKeeperClient(conf);
+ MetadataClientDriver metadataClientDriver =
bkc.getMetadataClientDriver();
+ metadataClientDriver.setConnectionExpiredListener(() -> {
Review comment:
I think a cleaner solution is AuditorElector handling this session expires.
because AuditorElector already has this session expires watcher to shutdown
elector -
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AuditorElector.java#L224
so all the leader election logic are all in one place, rather than spreading
across multiple places. and it seems to be working better with #1701
(https://github.com/apache/bookkeeper/pull/1701/files#diff-7d01af204bb0a30c0193b46c737f8cbfR49)
However I am fine with this change as it is working well with the zookeeper
concepts. We can go with this change for now, once the change #1701 is merged,
we can considering moving this session expires logic in to the AuditorElector,
which will provide a much cleaner abstraction.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
