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/08 08:51:43 UTC
[GitHub] [pulsar] baomingyu opened a new pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
baomingyu opened a new pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965
Fixes #11964
*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*
In key_shared mode. Ack message will hold PersistentSubscription(synchronized lock ) --> ManagedCursorImpl(lock) -> to try get PersistentStickyKeyDispatcherMultipleConsumers(synchronized) in asyncDelete function and when individualDeletedMessages is empty way. Same time , other Thread Ack message when individualDeletedMessages is not empty maybe hold PersistentStickyKeyDispatcherMultipleConsumers(synchronized) to try get ManagedCursorImpl(lock) for read more entries. deadlock will happen.
### Modifications
change unlock before callback ,in asyncDelete function of ManagedCursorImpl.
--
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] baomingyu commented on pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
baomingyu commented on pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965#issuecomment-919659506
> @baomingyu Nice catch, could you point me where is the `Ack message will hold PersistentSubscription(synchronized lock )`? The acknowledgeMessage() method
>
> https://github.com/apache/pulsar/blob/0584628cc1968f6b50110d8596d3828d0181db2b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java#L357
>
> in the PersistentSubscription will not hold the lock.
for one way : ack message --> asyncDelete (in ManagedCursorImpl ---get write lock) ---> then callback will try to get synchronized lock of PersistentStickyKeyDispatcherMultipleConsumers. Same time , other Thread Ack message when individualDeletedMessages is not empty maybe hold PersistentStickyKeyDispatcherMultipleConsumers(synchronized) to try get ManagedCursorImpl(lock) for read more entries. deadlock will happen.
--
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] Anonymitaet commented on pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965#issuecomment-915651027
@baomingyu Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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] tomscut commented on a change in pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
tomscut commented on a change in pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965#discussion_r722940221
##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
##########
@@ -2021,7 +2020,12 @@ public void asyncDelete(Iterable<Position> positions, AsyncCallbacks.DeleteCallb
callback.deleteFailed(getManagedLedgerException(e), ctx);
return;
} finally {
- lock.writeLock().unlock();
+ if (individualDeletedMessages.isEmpty()) {
Review comment:
Good catch. Can we change the code to something like this:
```
finally {
lock.writeLock().unlock();
if (individualDeletedMessages.isEmpty()) {
callback.deleteComplete(ctx);
}
}
```
--
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 pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965#issuecomment-915266595
@baomingyu Nice catch, could you point me where is the `Ack message will hold PersistentSubscription(synchronized lock )`? The acknowledgeMessage() method https://github.com/apache/pulsar/blob/0584628cc1968f6b50110d8596d3828d0181db2b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java#L357 in the PersistentSubscription will not hold the 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] codelipenghui merged pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965
--
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] baomingyu commented on pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
baomingyu commented on pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965#issuecomment-927433708
> @baomingyu Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
Docs will not be needed to update.
--
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] baomingyu commented on pull request #11965: fix issues 11964, deadlock bug when use key_shared mode
Posted by GitBox <gi...@apache.org>.
baomingyu commented on pull request #11965:
URL: https://github.com/apache/pulsar/pull/11965#issuecomment-930666834
/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