Skip to content

Commit b93176c

Browse files
committed
[fix][broker] Ignore and remove the replicator cursor when the remote cluster is absent (#19972)
(cherry picked from commit d1fc732)
1 parent cef4f71 commit b93176c

File tree

2 files changed

+90
-6
lines changed

2 files changed

+90
-6
lines changed

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,14 +1538,32 @@ public void openCursorFailed(ManagedLedgerException exception, Object ctx) {
15381538
return future;
15391539
}
15401540

1541+
private CompletableFuture<Boolean> checkReplicationCluster(String remoteCluster) {
1542+
return brokerService.getPulsar().getPulsarResources().getNamespaceResources()
1543+
.getPoliciesAsync(TopicName.get(topic).getNamespaceObject())
1544+
.thenApply(optPolicies -> optPolicies.map(policies -> policies.replication_clusters)
1545+
.orElse(Collections.emptySet()).contains(remoteCluster)
1546+
|| topicPolicies.getReplicationClusters().get().contains(remoteCluster));
1547+
}
1548+
15411549
protected CompletableFuture<Void> addReplicationCluster(String remoteCluster, ManagedCursor cursor,
15421550
String localCluster) {
15431551
return AbstractReplicator.validatePartitionedTopicAsync(PersistentTopic.this.getName(), brokerService)
1544-
.thenCompose(__ -> brokerService.pulsar().getPulsarResources().getClusterResources()
1545-
.getClusterAsync(remoteCluster)
1546-
.thenApply(clusterData ->
1547-
brokerService.getReplicationClient(remoteCluster, clusterData)))
1552+
.thenCompose(__ -> checkReplicationCluster(remoteCluster))
1553+
.thenCompose(clusterExists -> {
1554+
if (!clusterExists) {
1555+
log.warn("Remove the replicator because the cluster '{}' does not exist", remoteCluster);
1556+
return removeReplicator(remoteCluster).thenApply(__ -> null);
1557+
}
1558+
return brokerService.pulsar().getPulsarResources().getClusterResources()
1559+
.getClusterAsync(remoteCluster)
1560+
.thenApply(clusterData ->
1561+
brokerService.getReplicationClient(remoteCluster, clusterData));
1562+
})
15481563
.thenAccept(replicationClient -> {
1564+
if (replicationClient == null) {
1565+
return;
1566+
}
15491567
Replicator replicator = replicators.computeIfAbsent(remoteCluster, r -> {
15501568
try {
15511569
return new PersistentReplicator(PersistentTopic.this, cursor, localCluster,
@@ -1569,8 +1587,8 @@ CompletableFuture<Void> removeReplicator(String remoteCluster) {
15691587

15701588
String name = PersistentReplicator.getReplicatorName(replicatorPrefix, remoteCluster);
15711589

1572-
replicators.get(remoteCluster).disconnect().thenRun(() -> {
1573-
1590+
Optional.ofNullable(replicators.get(remoteCluster)).map(Replicator::disconnect)
1591+
.orElse(CompletableFuture.completedFuture(null)).thenRun(() -> {
15741592
ledger.asyncDeleteCursor(name, new DeleteCursorCallback() {
15751593
@Override
15761594
public void deleteCursorComplete(Object ctx) {

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PersistentTopicTest.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,32 +35,41 @@
3535
import com.google.common.collect.Sets;
3636
import java.lang.reflect.Field;
3737
import java.util.ArrayList;
38+
import java.util.Collections;
39+
import java.util.HashSet;
3840
import java.util.List;
41+
import java.util.Set;
3942
import java.util.UUID;
4043
import java.util.concurrent.TimeUnit;
4144
import lombok.Data;
4245
import java.util.concurrent.CompletableFuture;
4346
import java.util.concurrent.ExecutionException;
4447
import java.util.concurrent.atomic.AtomicBoolean;
48+
import java.util.function.Supplier;
4549
import org.apache.bookkeeper.client.LedgerHandle;
50+
import org.apache.bookkeeper.mledger.ManagedCursor;
4651
import org.apache.bookkeeper.mledger.ManagedLedger;
4752
import org.apache.pulsar.broker.service.BrokerService;
4853
import org.apache.pulsar.broker.service.BrokerTestBase;
4954
import org.apache.pulsar.client.api.Consumer;
5055
import org.apache.pulsar.client.api.Message;
56+
import org.apache.pulsar.client.api.MessageId;
5157
import org.apache.pulsar.client.api.MessageRoutingMode;
5258
import org.apache.pulsar.client.api.Producer;
5359
import org.apache.pulsar.client.api.PulsarClientException;
5460
import org.apache.pulsar.client.api.Schema;
5561
import org.apache.pulsar.client.api.SubscriptionType;
5662
import org.apache.pulsar.common.naming.NamespaceBundle;
5763
import org.apache.pulsar.common.naming.TopicName;
64+
import org.apache.pulsar.common.policies.data.ClusterData;
5865
import org.apache.pulsar.common.policies.data.Policies;
66+
import org.apache.pulsar.common.policies.data.TenantInfo;
5967
import org.apache.pulsar.common.policies.data.TopicStats;
6068
import org.awaitility.Awaitility;
6169
import org.testng.Assert;
6270
import org.testng.annotations.AfterMethod;
6371
import org.testng.annotations.BeforeMethod;
72+
import org.testng.annotations.DataProvider;
6473
import org.testng.annotations.Test;
6574

6675
@Test(groups = "broker")
@@ -69,6 +78,8 @@ public class PersistentTopicTest extends BrokerTestBase {
6978
@BeforeMethod(alwaysRun = true)
7079
@Override
7180
protected void setup() throws Exception {
81+
conf.setSystemTopicEnabled(true);
82+
conf.setTopicLevelPoliciesEnabled(true);
7283
super.baseSetup();
7384
}
7485

@@ -392,4 +403,59 @@ public void testDeleteTopicFail() throws Exception {
392403
makeDeletedFailed.set(false);
393404
persistentTopic.delete().get();
394405
}
406+
407+
@DataProvider(name = "topicLevelPolicy")
408+
public static Object[][] topicLevelPolicy() {
409+
return new Object[][] { { true }, { false } };
410+
}
411+
412+
@Test(dataProvider = "topicLevelPolicy")
413+
public void testCreateTopicWithZombieReplicatorCursor(boolean topicLevelPolicy) throws Exception {
414+
final String namespace = "prop/ns-abc";
415+
final String topicName = "persistent://" + namespace
416+
+ "/testCreateTopicWithZombieReplicatorCursor" + topicLevelPolicy;
417+
final String remoteCluster = "remote";
418+
admin.topics().createNonPartitionedTopic(topicName);
419+
admin.topics().createSubscription(topicName, conf.getReplicatorPrefix() + "." + remoteCluster,
420+
MessageId.earliest, true);
421+
422+
admin.clusters().createCluster(remoteCluster, ClusterData.builder()
423+
.serviceUrl("http://localhost:11112")
424+
.brokerServiceUrl("pulsar://localhost:11111")
425+
.build());
426+
TenantInfo tenantInfo = admin.tenants().getTenantInfo("prop");
427+
tenantInfo.getAllowedClusters().add(remoteCluster);
428+
admin.tenants().updateTenant("prop", tenantInfo);
429+
430+
if (topicLevelPolicy) {
431+
admin.topics().setReplicationClusters(topicName, Collections.singletonList(remoteCluster));
432+
} else {
433+
admin.namespaces().setNamespaceReplicationClustersAsync(
434+
namespace, Collections.singleton(remoteCluster)).get();
435+
}
436+
437+
final PersistentTopic topic = (PersistentTopic) pulsar.getBrokerService().getTopic(topicName, false)
438+
.get(3, TimeUnit.SECONDS).orElse(null);
439+
assertNotNull(topic);
440+
441+
final Supplier<Set<String>> getCursors = () -> {
442+
final Set<String> cursors = new HashSet<>();
443+
final Iterable<ManagedCursor> iterable = topic.getManagedLedger().getCursors();
444+
iterable.forEach(c -> cursors.add(c.getName()));
445+
return cursors;
446+
};
447+
assertEquals(getCursors.get(), Collections.singleton(conf.getReplicatorPrefix() + "." + remoteCluster));
448+
449+
if (topicLevelPolicy) {
450+
admin.topics().setReplicationClusters(topicName, Collections.emptyList());
451+
} else {
452+
admin.namespaces().setNamespaceReplicationClustersAsync(namespace, Collections.emptySet()).get();
453+
}
454+
admin.clusters().deleteCluster(remoteCluster);
455+
// Now the cluster and its related policy has been removed but the replicator cursor still exists
456+
457+
topic.initialize().get(3, TimeUnit.SECONDS);
458+
Awaitility.await().atMost(3, TimeUnit.SECONDS)
459+
.until(() -> !topic.getManagedLedger().getCursors().iterator().hasNext());
460+
}
395461
}

0 commit comments

Comments
 (0)