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 2022/04/20 17:37:51 UTC
[GitHub] [pulsar] Shawyeok opened a new pull request, #15241: [Broker] Refactor: arrange cleanup operation for topic creation
Shawyeok opened a new pull request, #15241:
URL: https://github.com/apache/pulsar/pull/15241
### Motivation
Original from https://github.com/apache/pulsar/pull/15139#issuecomment-1098692062.
If create topic fails, many code branch handle cleanup operation for `topics` map like `pulsar.getExecutor().execute(() -> topics.remove(topic, topicFuture));`, handle it in separate thread to avoid deadlock, and some code branch doesn't. IMO, it should be handled properly in just one place.
### Modifications
1. handle cleanup operation before return future result
2. delete other cleanup operation except one with specific reason https://github.com/Shawyeok/pulsar/blob/78bbbe66d5abf66fc0c6ce15624107227a17a360/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1190
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [x] `no-need-doc`
Just refactor work
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15241: [Broker] Refactor: arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#issuecomment-1133802288
The pr had no activity for 30 days, mark with Stale label.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15241: [refactor][broker] Arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#issuecomment-1236255515
@Shawyeok Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #15241: [refactor][broker] Arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#issuecomment-1345273651
Closed as stale and conflict. Please rebase and resubmit the patch if it's still relevant.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15241: [Broker] Refactor: arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#issuecomment-1179639518
The pr had no activity for 30 days, mark with Stale label.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #15241: [Broker] Refactor: arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#discussion_r855115500
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -1432,9 +1434,7 @@ public void openLedgerComplete(ManagedLedger ledger, Object ctx) {
if (topicFuture.isCompletedExceptionally()) {
log.warn("{} future is already completed with failure {}, closing the"
+ " topic", topic, FutureUtil.getException(topicFuture));
- persistentTopic.stopReplProducers().whenComplete((v, exception) -> {
Review Comment:
In original implements, It ensures that `persistentTopic.stopReplProducers()` executes before `topics.remove(...)`, this ensures that "there are no multiple threads change one topic"
After this change, this advantage was deleted.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #15241: [Broker] Refactor: arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#discussion_r855117708
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -1432,9 +1434,7 @@ public void openLedgerComplete(ManagedLedger ledger, Object ctx) {
if (topicFuture.isCompletedExceptionally()) {
log.warn("{} future is already completed with failure {}, closing the"
+ " topic", topic, FutureUtil.getException(topicFuture));
- persistentTopic.stopReplProducers().whenComplete((v, exception) -> {
- topics.remove(topic, topicFuture);
- });
+ persistentTopic.stopReplProducers();
Review Comment:
In original implements, It ensures that `persistentTopic.stopReplProducers()` executes before `topics.remove(...)`, this ensures that "there are no multiple threads change one topic"
After this change, this advantage was deleted.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #15241: [Broker] Refactor: arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15241:
URL: https://github.com/apache/pulsar/pull/15241#discussion_r855114449
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -1432,9 +1434,7 @@ public void openLedgerComplete(ManagedLedger ledger, Object ctx) {
if (topicFuture.isCompletedExceptionally()) {
log.warn("{} future is already completed with failure {}, closing the"
+ " topic", topic, FutureUtil.getException(topicFuture));
- persistentTopic.stopReplProducers().whenComplete((v, exception) -> {
Review Comment:
In original implements, It ensures that `persistentTopic.stopReplProducers()` executes before `topics.remove(...)`, this ensures that "there are no multiple threads change one topic"
After this change, this advantage was deleted.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] tisonkun closed pull request #15241: [refactor][broker] Arrange cleanup operation for topic creation
Posted by GitBox <gi...@apache.org>.
tisonkun closed pull request #15241: [refactor][broker] Arrange cleanup operation for topic creation
URL: https://github.com/apache/pulsar/pull/15241
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org