Skip to content

Commit

Permalink
[fix][broker]Fix deadlock of metadata store (apache#20189)
Browse files Browse the repository at this point in the history
Motivation: This task loadOrCreatePersistentTopic occupied the event thread of the ZK client so that other ZK tasks could not be finished anymore(Including the task itself), and it calls bundlesCache.synchronous().get(nsname) which is a blocking method.

Modification: Since the method getBundle(topic) will eventually call the method bundlesCache.synchronous().get(nsname), use getBundleAsync(topic) instead of getBundle(topic) to avoid blocking the thread.
  • Loading branch information
poorbarcode authored May 18, 2023
1 parent e53fcaa commit 4678c36
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -1137,16 +1139,17 @@ public CompletableFuture<Boolean> isServiceUnitOwnedAsync(ServiceUnitId suName)
new IllegalArgumentException("Invalid class of NamespaceBundle: " + suName.getClass().getName()));
}

/**
* @Deprecated This method is only used in test now.
*/
@Deprecated
public boolean isServiceUnitActive(TopicName topicName) {
try {
OwnedBundle ownedBundle = ownershipCache.getOwnedBundle(getBundle(topicName));
if (ownedBundle == null) {
return false;
}
return ownedBundle.isActive();
} catch (Exception e) {
LOG.warn("Unable to find OwnedBundle for topic - [{}]", topicName, e);
return false;
return isServiceUnitActiveAsync(topicName).get(pulsar.getConfig()
.getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException | ExecutionException | TimeoutException e) {
LOG.warn("Unable to find OwnedBundle for topic in time - [{}]", topicName, e);
throw new RuntimeException(e);
}
}

Expand All @@ -1156,12 +1159,13 @@ public CompletableFuture<Boolean> isServiceUnitActiveAsync(TopicName topicName)
return getBundleAsync(topicName)
.thenCompose(bundle -> loadManager.get().checkOwnershipAsync(Optional.of(topicName), bundle));
}
Optional<CompletableFuture<OwnedBundle>> res = ownershipCache.getOwnedBundleAsync(getBundle(topicName));
if (!res.isPresent()) {
return CompletableFuture.completedFuture(false);
}

return res.get().thenApply(ob -> ob != null && ob.isActive());
return getBundleAsync(topicName).thenCompose(bundle -> {
Optional<CompletableFuture<OwnedBundle>> optionalFuture = ownershipCache.getOwnedBundleAsync(bundle);
if (!optionalFuture.isPresent()) {
return CompletableFuture.completedFuture(false);
}
return optionalFuture.get().thenApply(ob -> ob != null && ob.isActive());
});
}

private boolean isNamespaceOwned(NamespaceName fqnn) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,10 @@ public Optional<CompletableFuture<OwnedBundle>> getOwnedBundleAsync(NamespaceBun

/**
* Disable bundle in local cache and on zk.
*
* @param bundle
* @throws Exception
* @Deprecated This is a dangerous method which is currently only used for test, it will occupy the ZK thread.
* Please switch to your own thread after calling this method.
*/
@Deprecated
public CompletableFuture<Void> disableOwnership(NamespaceBundle bundle) {
return updateBundleState(bundle, false)
.thenCompose(__ -> {
Expand Down

0 comments on commit 4678c36

Please sign in to comment.