devinbost opened a new pull request #11252:
URL: https://github.com/apache/pulsar/pull/11252
**Motivation:**
Investigation of #6054 has revealed cases where brokers get stuck where
`havePendingRead = true` on `PersistentDispatcherMultipleConsumers` without
actually reading from Bookkeeper. This indicates a race may be happening on
`havePendingRead`.
**Implementation:**
It looks like this access and write could be thread-unsafe since
`havePendingRead` is volatile:
```
@Override
protected void cancelPendingRead() {
if (havePendingRead && cursor.cancelPendingReadRequest()) {
havePendingRead = false;
}
}
```
Two threads could concurrently read `havePendingRead = true` even though one
thread is going to cancel the read request. If the read request is canceled
after `havePendingRead` is set to `true`, it appears that we could skip the
call to `readMoreEntries()`
(https://github.com/apache/pulsar/blob/945b1b68452da6f5ec2abc671db31c7d94a2b8e6/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L646)
when we're expecting to retry the read.
All other methods in `PersistentDispatcherMultipleConsumers` that interact
with `havePendingRead` are synchronized.
A couple of other options would be to make the boolean an `AtomicBoolean` or
use an `AtomicIntegerFieldUpdater`. However, the other methods are already
synchronized, and in the way `cancelPendingRead()` is written, we need a
thread-safe way to check that `havePendingRead` is true before canceling the
pending read request on the cursor and then setting `havePendingRead = false`.
So, we could use a local lock here, but that wouldn't guarantee the safety of
`havePendingRead`.
So, it seems that synchronizing `cancelPendingRead()` is the best approach.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
