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/09/29 14:48:56 UTC
[GitHub] [pulsar] Shoothzj opened a new pull request #12240: fix the race of delete subscription and delete topic
Shoothzj opened a new pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240
Fixes #12239
### Motivation
See #12239
*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*
### Modifications
*Describe the modifications you've done.*
### Documentation
Need to update docs?
- [ ] no-need-doc
Bug fix, no need doc
--
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] codelipenghui merged pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240
--
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] Shoothzj removed a comment on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-950119240
/pulsarbot run-failure-checks
--
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] Shoothzj removed a comment on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-950241478
/pulsarbot run-failure-checks
--
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] codelipenghui commented on a change in pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#discussion_r725487937
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -1006,24 +1006,33 @@ public void deleteLedgerFailed(ManagedLedgerException exception, Object ctx) {
}
private void asyncDeleteCursor(String subscriptionName, CompletableFuture<Void> unsubscribeFuture) {
+ lock.writeLock().lock();
ledger.asyncDeleteCursor(Codec.encode(subscriptionName), new DeleteCursorCallback() {
@Override
public void deleteCursorComplete(Object ctx) {
- if (log.isDebugEnabled()) {
- log.debug("[{}][{}] Cursor deleted successfully", topic, subscriptionName);
+ try {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}][{}] Cursor deleted successfully", topic, subscriptionName);
+ }
+ removeSubscription(subscriptionName);
+ unsubscribeFuture.complete(null);
+ lastActive = System.nanoTime();
+ } finally {
+ lock.writeLock().unlock();
}
- removeSubscription(subscriptionName);
- unsubscribeFuture.complete(null);
- lastActive = System.nanoTime();
}
@Override
public void deleteCursorFailed(ManagedLedgerException exception, Object ctx) {
- if (log.isDebugEnabled()) {
- log.debug("[{}][{}] Error deleting cursor for subscription",
- topic, subscriptionName, exception);
+ try {
+ if (log.isDebugEnabled()) {
+ log.debug("[{}][{}] Error deleting cursor for subscription",
+ topic, subscriptionName, exception);
+ }
+ unsubscribeFuture.completeExceptionally(new PersistenceException(exception));
Review comment:
We'd better to handle the cursor not found exception to avoid introduce write lock.
--
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] Shoothzj removed a comment on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-942995060
/pulsarbot run-failure-checks
--
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] Shoothzj commented on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-930251481
@codelipenghui could you please review this modification, and give me some idea to add tests.
Thank you :)
--
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] Shoothzj commented on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-950415095
/pulsarbot run-failure-checks
--
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] Shoothzj removed a comment on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-950415095
/pulsarbot run-failure-checks
--
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] Shoothzj commented on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-930251481
@codelipenghui could you please review this modification, and give me some idea to add tests.
Thank you :)
--
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] Shoothzj commented on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-950119240
/pulsarbot run-failure-checks
--
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] Shoothzj commented on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-950241478
/pulsarbot run-failure-checks
--
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] Shoothzj commented on pull request #12240: fix the race of delete subscription and delete topic
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on pull request #12240:
URL: https://github.com/apache/pulsar/pull/12240#issuecomment-942995060
/pulsarbot run-failure-checks
--
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