merlimat commented on a change in pull request #9286:
URL: https://github.com/apache/pulsar/pull/9286#discussion_r562759077
##########
File path:
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/AbstractMetadataStore.java
##########
@@ -207,7 +207,7 @@ public void accept(Notification n) {
@Override
public void close() throws Exception {
- executor.shutdownNow();
+ executor.shutdown();
Review comment:
Why do we need this change? The `shutdownNow()` will ensure that the
thread is interrupted if it was waiting on anything. I think that, as a general
rule, it should be preferable to wait for the thread to exit on its own.
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java
##########
@@ -542,8 +543,17 @@ public void closeFailed(ManagedLedgerException exception,
Object ctx) {
}
scheduledExecutor.shutdownNow();
- orderedExecutor.shutdownNow();
+ if (!scheduledExecutor.awaitTermination(30, TimeUnit.SECONDS)) {
+ log.error("Cannot stop scheduledExecutor gracefully.");
+ }
+ orderedExecutor.shutdown();
Review comment:
Why not:
```suggestion
orderedExecutor.shutdownNow();
```
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java
##########
@@ -512,6 +512,7 @@ public void shutdown() throws InterruptedException,
ManagedLedgerException {
ledger.asyncClose(new AsyncCallbacks.CloseCallback() {
@Override
public void closeComplete(Object ctx) {
+ log.debug("[{}] Closed managed ledger.", ledger.getName());
Review comment:
Same as above
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java
##########
@@ -408,7 +408,7 @@ public void initializeFailed(ManagedLedgerException e) {
pendingLedger.ledger.asyncClose(new CloseCallback() {
@Override
public void closeComplete(Object ctx) {
- // no-op
+ log.debug("[{}] Closed pending initializer
managed ledger.", name);
Review comment:
`if (log.isDebugEnabled()) { ...} `
----------------------------------------------------------------
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]