sijie commented on a change in pull request #1809: Change LedgerManager to use
CompletableFuture
URL: https://github.com/apache/bookkeeper/pull/1809#discussion_r233127634
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java
##########
@@ -108,64 +115,69 @@ private GenericCallback removeCallback(GenericCallback
callback) {
return callbacks.remove(callback);
}
+ private void recordPromise(CompletableFuture<?> promise) {
Review comment:
I am not sure we need this or not. The underlying LM should error about its
complete futures, so all the complete futures at this class will be errored out
correctly. The change here is masking any potentially errors in underlying LM.
I would suggest not adding such logic for now. or at least don't do it in a
refactor PR.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
