From f48bceb174fedfa46c658eb864b65edbbf1504f1 Mon Sep 17 00:00:00 2001 From: Andrey Yegorov Date: Fri, 30 Sep 2022 12:02:59 -0700 Subject: [PATCH 1/2] Do not call blocking close() in the callback --- .../bookkeeper/client/LedgerOpenOp.java | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java index 3df610c936e..e64320544ee 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java @@ -28,6 +28,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantReadWriteLock; +import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.client.AsyncCallback.OpenCallback; import org.apache.bookkeeper.client.AsyncCallback.ReadLastConfirmedCallback; import org.apache.bookkeeper.client.BookKeeper.DigestType; @@ -47,6 +48,7 @@ * Encapsulates the ledger open operation. * */ +@Slf4j class LedgerOpenOp { static final Logger LOG = LoggerFactory.getLogger(LedgerOpenOp.class); @@ -128,17 +130,11 @@ public void initiateWithoutRecovery() { initiate(); } - private void closeLedgerHandle() { - try { - if (lh != null) { - lh.close(); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - LOG.info("InterruptedException while closing ledger {}", ledgerId, e); - } catch (BKException e) { - LOG.warn("BKException while closing ledger {} ", ledgerId, e); + private CompletableFuture closeLedgerHandleAsync() { + if (lh != null) { + return lh.closeAsync(); } + return CompletableFuture.completedFuture(null); } private void openWithMetadata(Versioned versionedMetadata) { @@ -208,10 +204,13 @@ public void safeOperationComplete(int rc, Void result) { if (rc == BKException.Code.OK) { openComplete(BKException.Code.OK, lh); } else if (rc == BKException.Code.UnauthorizedAccessException) { - closeLedgerHandle(); - openComplete(BKException.Code.UnauthorizedAccessException, null); + closeLedgerHandleAsync().whenComplete((r, ex) -> { + if (ex != null) { + log.error("Ledger {} close failed", ledgerId, ex); + } + openComplete(BKException.Code.UnauthorizedAccessException, null); + }); } else { - closeLedgerHandle(); openComplete(bk.getReturnRc(BKException.Code.LedgerRecoveryException), null); } } @@ -226,8 +225,12 @@ public String toString() { public void readLastConfirmedComplete(int rc, long lastConfirmed, Object ctx) { if (rc != BKException.Code.OK) { - closeLedgerHandle(); - openComplete(bk.getReturnRc(BKException.Code.ReadException), null); + closeLedgerHandleAsync().whenComplete((r, ex) -> { + if (ex != null) { + log.error("Ledger {} close failed", ledgerId, ex); + } + openComplete(bk.getReturnRc(BKException.Code.ReadException), null); + }); } else { lh.lastAddConfirmed = lh.lastAddPushed = lastConfirmed; openComplete(BKException.Code.OK, lh); From 2502b35b0ea0b2ba4208d6879e88391111f0b695 Mon Sep 17 00:00:00 2001 From: Andrey Yegorov Date: Mon, 3 Oct 2022 10:51:59 -0700 Subject: [PATCH 2/2] CR feedback, remove lombok from LedgerOpenOp --- .../java/org/apache/bookkeeper/client/LedgerOpenOp.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java index e64320544ee..584e17ce72f 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerOpenOp.java @@ -28,7 +28,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantReadWriteLock; -import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.client.AsyncCallback.OpenCallback; import org.apache.bookkeeper.client.AsyncCallback.ReadLastConfirmedCallback; import org.apache.bookkeeper.client.BookKeeper.DigestType; @@ -48,7 +47,6 @@ * Encapsulates the ledger open operation. * */ -@Slf4j class LedgerOpenOp { static final Logger LOG = LoggerFactory.getLogger(LedgerOpenOp.class); @@ -206,7 +204,7 @@ public void safeOperationComplete(int rc, Void result) { } else if (rc == BKException.Code.UnauthorizedAccessException) { closeLedgerHandleAsync().whenComplete((r, ex) -> { if (ex != null) { - log.error("Ledger {} close failed", ledgerId, ex); + LOG.error("Ledger {} close failed", ledgerId, ex); } openComplete(BKException.Code.UnauthorizedAccessException, null); }); @@ -227,7 +225,7 @@ public void readLastConfirmedComplete(int rc, if (rc != BKException.Code.OK) { closeLedgerHandleAsync().whenComplete((r, ex) -> { if (ex != null) { - log.error("Ledger {} close failed", ledgerId, ex); + LOG.error("Ledger {} close failed", ledgerId, ex); } openComplete(bk.getReturnRc(BKException.Code.ReadException), null); });