Skip to content

Conversation

@BewareMyPower
Copy link
Contributor

Motivation

The modifications on
AbstractDispatcherSingleActiveConsumer#activeConsumer only happen on pickAndScheduleActiveConsumer, which is only called in addConsumer and removeConsumer. However, both addConsumer and removeConsumer are synchronized methods so that we don't have to perform an atomic operation on activeConsumer.

The composite operations on activeConsumer also happen on synchronized methods like disconnectActiveConsumers. The only access to activeConsumer that is not synchronized is the read on activeConsumer like getActiveConsumer() and isConsumerConnected().

Modifications

Remove the ACTIVE_CONSUMER_UPDATER and note some methods are not thread safe or there is no need to make them thread safe.

Besides, since pickAndScheduleActiveConsumer is never called concurrently, we don't need a double-checked locking on readOnActiveConsumerTask. Instead, just synchronize the modification in the scheduled task.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

…cher

### Motivation

The modifications on
`AbstractDispatcherSingleActiveConsumer#activeConsumer` only happen on
`pickAndScheduleActiveConsumer`, which is only called in `addConsumer`
and `removeConsumer`. However, both `addConsumer` and `removeConsumer`
are synchronized methods so that we don't have to perform an atomic
operation on `activeConsumer`.

The composite operations on `activeConsumer` also happen on synchronized
methods like `disconnectActiveConsumers`. The only access to
`activeConsumer` that is not synchronized is the read on
`activeConsumer` like `getActiveConsumer()` and `isConsumerConnected()`.

### Modifications

Remove the `ACTIVE_CONSUMER_UPDATER` and note some methods are not
thread safe or there is no need to make them thread safe.

Besides, since `pickAndScheduleActiveConsumer` is never called
concurrently, we don't need a double-checked locking on
`readOnActiveConsumerTask`. Instead, just synchronize the modification
in the scheduled task.
@BewareMyPower BewareMyPower added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker labels Mar 16, 2024
@BewareMyPower BewareMyPower added this to the 3.3.0 milestone Mar 16, 2024
@BewareMyPower BewareMyPower self-assigned this Mar 16, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 16, 2024
@Technoboy- Technoboy- merged commit 1b1bd4b into apache:master Mar 19, 2024
@BewareMyPower BewareMyPower deleted the bewaremypower/active-consumer-thread-safety branch March 19, 2024 11:53
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs ready-to-test type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants