You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/07 07:34:11 UTC
[GitHub] [pulsar] rdhabalia opened a new pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
rdhabalia opened a new pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847
### Motivation
When the broker takes a longer time to load the topic and times out before completing the topic future, then the broker keeps multiple topics opened , doesn't clean up timed out topic, fail to create replicator producer on successfully created topic with error: `repl-producer is already connected to topic`, builds replication backlog.
```
19:16:10.107 [pulsar-ordered-OrderedExecutor-5-0] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - Opening managed ledger myProp/global/myNs/persistent/myTopic
:
9:17:10.953 [BookKeeperClientWorker-OrderedExecutor-2-0] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl - [myProp/global/myNs/persistent/myTopic] Successfully initialize managed ledger
:
19:17:10.065 [pulsar-io-23-30] ERROR org.apache.pulsar.broker.service.ServerCnx - [/10.196.133.62:47278] Failed to create topic persistent://myProp/global/myNs/myTopic, producerId=382
:
19:17:10.954 [BookKeeperClientWorker-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.persistent.PersistentReplicator - [persistent://myProp/global/myNs/myTopic][west1 -> west2] Starting open cursor for replicator
:
19:17:10.955 [BookKeeperClientWorker-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.BrokerService - Created topic persistent://myProp/global/myNs/myTopic - dedup is disabled
:
19:17:51.532 [pulsar-ordered-OrderedExecutor-5-0] INFO org.apache.pulsar.broker.service.BrokerService - Created topic persistent://myProp/global/myNs/myTopic - dedup is disabled
:
19:17:51.530 [pulsar-ordered-OrderedExecutor-5-0] INFO org.apache.pulsar.broker.service.persistent.PersistentReplicator - [persistent://myProp/global/myNs/myTopic][west1 -> west2] Starting open cursor for replicator
:
07:25:51.377 [pulsar-io-23-5] ERROR org.apache.pulsar.client.impl.ProducerImpl - [persistent://myProp/global/myNs/myTopic] [pulsar.repl.west1] Failed to create producer: Producer with name 'pulsarrepl.west1' is already connected to topic
```
### Modification
- Stopped replicator for failed and timed-out topic
- Clean up failed topic
### Result
- Successfully create replicator producer for the topic and avoid creating replication backlog
--
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:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#issuecomment-856103684
yes, we can move to `2.8.1`
--
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:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#issuecomment-856964104
@Anonymitaet No, this is a bug fix and we don't need documentation for this one.
--
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:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#issuecomment-855730787
@rdhabalia we have started the 2.8.0 release, can we move it 2.8.1 or 2.7.3?
--
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:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#issuecomment-856646801
@rdhabalia thanks for your contribution. For this PR, do we need to update docs?
--
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:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 commented on a change in pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on a change in pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#discussion_r647196841
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java
##########
@@ -162,4 +164,33 @@ public synchronized Throwable fillInStackTrace() {
return this;
}
}
+
+ public static <T> CompletableFuture<T> futureWithDeadline(ScheduledExecutorService executor, Long delay,
Review comment:
What is the difference between this and ```addTimeoutHandling```? why we need add this method?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -970,7 +970,7 @@ public void deleteLedgerFailed(ManagedLedgerException exception, Object ctx) {
}
private CompletableFuture<Optional<Topic>> createNonPersistentTopic(String topic) {
- CompletableFuture<Optional<Topic>> topicFuture = futureWithDeadline();
+ CompletableFuture<Optional<Topic>> topicFuture = FutureUtil.futureWithDeadline(executor());
Review comment:
why need change ```futureWithDeadline()``` to ```FutureUtil.futureWithDeadline(executor())```?
--
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:
users@infra.apache.org
[GitHub] [pulsar] sijie merged pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
sijie merged pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847
--
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:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on a change in pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#discussion_r647656730
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -970,7 +970,7 @@ public void deleteLedgerFailed(ManagedLedgerException exception, Object ctx) {
}
private CompletableFuture<Optional<Topic>> createNonPersistentTopic(String topic) {
- CompletableFuture<Optional<Topic>> topicFuture = futureWithDeadline();
+ CompletableFuture<Optional<Topic>> topicFuture = FutureUtil.futureWithDeadline(executor());
Review comment:
It is a utility method for Future so, it should be part of `FutureUtil`
--
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:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on a change in pull request #10847: [pulsar-broker] Handle multiple topic creation for same topic-name in broker
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #10847:
URL: https://github.com/apache/pulsar/pull/10847#discussion_r647656487
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java
##########
@@ -162,4 +164,33 @@ public synchronized Throwable fillInStackTrace() {
return this;
}
}
+
+ public static <T> CompletableFuture<T> futureWithDeadline(ScheduledExecutorService executor, Long delay,
Review comment:
it is an existing method just moved to `FutureUtil` class. `futureWithDeadline` completes future with exception after the time whereas timeout handling something gets trigger once future is completed with exception.
--
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:
users@infra.apache.org