addisonj commented on a change in pull request #7506:
URL: https://github.com/apache/pulsar/pull/7506#discussion_r453069297
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java
##########
@@ -320,18 +335,32 @@ public void asyncOpen(final String name, final
ManagedLedgerConfig config, final
// If the ledger state is bad, remove it from the map.
CompletableFuture<ManagedLedgerImpl> existingFuture =
ledgers.get(name);
- if (existingFuture != null && existingFuture.isDone()) {
- try {
- ManagedLedgerImpl l = existingFuture.get();
- if (l.getState().equals(State.Fenced.toString()) ||
l.getState().equals(State.Closed.toString())) {
- // Managed ledger is in unusable state. Recreate it.
- log.warn("[{}] Attempted to open ledger in {} state.
Removing from the map to recreate it", name,
+ if (existingFuture != null) {
+ if (existingFuture.isDone()) {
+ try {
+ ManagedLedgerImpl l = existingFuture.get();
+ if (l.getState().equals(State.Fenced.toString()) ||
l.getState().equals(State.Closed.toString())) {
+ // Managed ledger is in unusable state. Recreate it.
+ log.warn("[{}] Attempted to open ledger in {} state.
Removing from the map to recreate it", name,
l.getState());
- ledgers.remove(name, existingFuture);
+ ledgers.remove(name, existingFuture);
+ }
+ } catch (Exception e) {
+ // Unable to get the future
+ log.warn("[{}] Got exception while trying to retrieve
ledger", name, e);
}
- } catch (Exception e) {
- // Unable to get the future
- log.warn("[{}] Got exception while trying to retrieve ledger",
name, e);
+ } else {
+ PendingInitializeManagedLedger pendingLedger =
pendingInitializeLedgers.get(name);
Review comment:
As opposed to requiring another call to check if the timeout has
elapsed, would it make sense to instead use a CompletableFuture with a timeout?
Much like we do in the BrokerService, the future created on line 370 could
instead be made to have a timeout if it isn't resolved in so many milliseconds
with the error handler remove the future from the cache of futures.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
