dlg99 commented on code in PR #3554:
URL: https://github.com/apache/bookkeeper/pull/3554#discussion_r1006041386
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java:
##########
@@ -737,6 +737,40 @@ public boolean isLedgerReplicationEnabled()
}
}
+ @Override
+ public void notifyLedgerReplicationStatusChanged(final
GenericCallback<Void> cb)
+ throws ReplicationException.UnavailableException {
Review Comment:
ReplicationException.fromKeeperException can throw
NonRecoverableReplicationException
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java:
##########
@@ -818,11 +821,6 @@ public void run() {
LOG.error("Exception running periodic check", bke);
} catch (IOException ioe) {
LOG.error("I/O exception running periodic check", ioe);
- } catch
(ReplicationException.NonRecoverableReplicationException nre) {
- LOG.error("Non Recoverable Exception while reading
from ZK", nre);
- submitShutdownTask();
Review Comment:
why is this removed / where NonRecoverableReplicationException is handled
now?
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java:
##########
@@ -1328,18 +1326,8 @@ void checkAllLedgers() throws BKException, IOException,
InterruptedException {
final CompletableFuture<Void> processFuture = new
CompletableFuture<>();
Processor<Long> checkLedgersProcessor = (ledgerId, callback) -> {
- try {
- if
(!ledgerUnderreplicationManager.isLedgerReplicationEnabled()) {
- LOG.info("Ledger rereplication has been disabled,
aborting periodic check");
- FutureUtils.complete(processFuture, null);
- return;
- }
- } catch
(ReplicationException.NonRecoverableReplicationException nre) {
- LOG.error("Non Recoverable Exception while reading from
ZK", nre);
- submitShutdownTask();
- return;
Review Comment:
why is this removed / where NonRecoverableReplicationException is handled
now?
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerUnderreplicationManager.java:
##########
@@ -148,6 +148,16 @@ void enableLedgerReplication()
boolean isLedgerReplicationEnabled()
throws ReplicationException.UnavailableException;
+ /**
+ * Receive notification asynchronously when the ledger replication process
+ * is changed.
+ *
+ * @param cb
+ * - callback implementation to receive the notification
+ */
+ void notifyLedgerReplicationStatusChanged(GenericCallback<Void> cb)
Review Comment:
something like
``` java
default void notifyLedgerReplicationStatusChanged(GenericCallback<Void> cb)
throws ReplicationException.UnavailableException {
throw new RuntimeException("not implemented");
}
```
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java:
##########
@@ -167,6 +167,7 @@ public class Auditor implements AutoCloseable {
private final int zkOpTimeoutMs;
private final Semaphore openLedgerNoRecoverySemaphore;
private final int openLedgerNoRecoverySemaphoreWaitTimeoutMSec;
+ private boolean isLedgerReplicationEnabled;
Review Comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
