You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2023/10/06 10:35:58 UTC
[I] [Task] Fix thread safety of PendingAckHandleImpl [pulsar]
lhotari opened a new issue, #21304:
URL: https://github.com/apache/pulsar/issues/21304
PendingAckHandleImpl seems to contain some thread safety issues:
individualAckOfTransaction uses org.apache.commons.collections4.map.LinkedMap which is not thread safe.
Another thread safety issue is the mutation of MutablePair instance. MutablePair is not thread safe.
HashMaps are also used as mutable state.
This state is mutated without a guarantee that the same thread is reading and writing the state or there's some other way to ensure consistency according to the Java Memory Model.
It seems that the intention would be use a single thread execution model to address this. There's a internalPinnedExecutor in
[PendingAckHandleImpl](https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/PendingAckHandleImpl.java) which is used to run some operations on the single executor thread.
However, the thread could possibly get switched multiple times in the call chain because of asynchronous method calls.
There's also some code where synchronized blocks are used as a way to ensure consistency such as
`synchronized (org.apache.pulsar.broker.transaction.pendingack.impl.PendingAckHandleImpl.this) {` and `synchronized (PendingAckHandleImpl.this) {`.
This is bad since the thread that gets synchronized is whatever thread is completing the asynchronous call. This might for example block threads from the bookkeeper client.
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Task] Fix thread safety of PendingAckHandleImpl [pulsar]
Posted by "JooHyukKim (via GitHub)" <gi...@apache.org>.
JooHyukKim commented on issue #21304:
URL: https://github.com/apache/pulsar/issues/21304#issuecomment-1800586369
If the odds are not high that thread safety issue will happen, maybe no need to be proactive on this, but at least can set up mechanism to easily catch up on this.
--
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
Re: [I] [Task] Fix thread safety of PendingAckHandleImpl [pulsar]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #21304:
URL: https://github.com/apache/pulsar/issues/21304#issuecomment-1797148160
The issue 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
Re: [I] [Task] Fix thread safety of PendingAckHandleImpl [pulsar]
Posted by "codelipenghui (via GitHub)" <gi...@apache.org>.
codelipenghui commented on issue #21304:
URL: https://github.com/apache/pulsar/issues/21304#issuecomment-1751698122
@liangyepianzhou Could you please help take a look at this issue?
--
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
Re: [I] [Task] Fix thread safety of PendingAckHandleImpl [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on issue #21304:
URL: https://github.com/apache/pulsar/issues/21304#issuecomment-1963544199
> If the odds are not high that thread safety issue will happen, maybe no need to be proactive on this, but at least can set up mechanism to easily catch up on this.
Perhaps in some software that's an acceptable way. Thread safety problems could be extremely rate. Let's say 1 out of 10 million operations fails. That is still a big problem for a messaging system.
--
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