Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[fix][broker] Fix NPE after publishing a tombstone to the service unit channel #22859

Merged

Conversation

BewareMyPower
Copy link
Contributor

Motivation

NPE will happen in UnloadManager#handleEvent after #22743. It's because the Init state is always associated with a null ServiceUnitStateData.

java.lang.NullPointerException: Cannot invoke "org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateData.force()" because "data" is null
        at org.apache.pulsar.broker.loadbalance.extensions.manager.UnloadManager.handleEvent(UnloadManager.java:204) ~[classes/:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.StateChangeListeners.lambda$notify$3(StateChangeListeners.java:74) ~[classes/:?]
        at java.base/java.util.concurrent.CopyOnWriteArrayList.forEach(CopyOnWriteArrayList.java:807) ~[?:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.StateChangeListeners.notify(StateChangeListeners.java:72) ~[classes/:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateChannelImpl.handleInitEvent(ServiceUnitStateChannelImpl.java:902) ~[classes/:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateChannelImpl.handle(ServiceUnitStateChannelImpl.java:715) ~[classes/:?]

Modifications

In UnloadManager#handleEvent, assume data is null and call complete directly. Fix UnloadManagerTest, which passes a non-null ServiceUnitStateData and Init to handleEvent.

Documentation

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

Matching PR in forked repository

PR in forked repository:

…t channel

### Motivation

NPE will happen in `UnloadManager#handleEvent` after
apache#22743. It's because the `Init`
state is always associated with a null `ServiceUnitStateData`.

```
java.lang.NullPointerException: Cannot invoke "org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateData.force()" because "data" is null
        at org.apache.pulsar.broker.loadbalance.extensions.manager.UnloadManager.handleEvent(UnloadManager.java:204) ~[classes/:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.StateChangeListeners.lambda$notify$3(StateChangeListeners.java:74) ~[classes/:?]
        at java.base/java.util.concurrent.CopyOnWriteArrayList.forEach(CopyOnWriteArrayList.java:807) ~[?:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.StateChangeListeners.notify(StateChangeListeners.java:72) ~[classes/:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateChannelImpl.handleInitEvent(ServiceUnitStateChannelImpl.java:902) ~[classes/:?]
        at org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateChannelImpl.handle(ServiceUnitStateChannelImpl.java:715) ~[classes/:?]
```

### Modifications

In `UnloadManager#handleEvent`, assume `data` is null and call
`complete` directly. Fix `UnloadManagerTest`, which passes a non-null
`ServiceUnitStateData` and `Init` to `handleEvent`.
@BewareMyPower BewareMyPower added this to the 3.4.0 milestone Jun 6, 2024
@BewareMyPower BewareMyPower self-assigned this Jun 6, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 6, 2024
@heesung-sn
Copy link
Contributor

Thank you for catching this.

@heesung-sn heesung-sn closed this Jun 6, 2024
@heesung-sn heesung-sn reopened this Jun 6, 2024
@heesung-sn
Copy link
Contributor

@BewareMyPower
Can you fix the check style error?

[INFO] There is 1 error reported by Checkstyle 10.14.2 with /home/runner/work/pulsar/pulsar/buildtools/src/main/resources/pulsar/checkstyle.xml ruleset.
Error:  src/main/java/org/apache/pulsar/broker/loadbalance/extensions/manager/UnloadManager.java:[25,1] (imports) ImportOrder: Import com.google.common.base.Preconditions.checkArgument appears after other imports that it should precede

@BewareMyPower
Copy link
Contributor Author

The checkstyle issue is fixed now.

@BewareMyPower BewareMyPower merged commit 9326a08 into apache:master Jun 7, 2024
50 of 51 checks passed
@BewareMyPower BewareMyPower deleted the bewaremypower/extensible-lb-npe branch June 7, 2024 03:09
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
…t channel (apache#22859)

(cherry picked from commit 9326a08)
(cherry picked from commit 6eac7e5)
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 26, 2024
…t channel (apache#22859)

(cherry picked from commit 9326a08)
(cherry picked from commit 6eac7e5)
heesung-sn pushed a commit to heesung-sn/pulsar that referenced this pull request Jun 27, 2024
…t channel (apache#22859)

(cherry picked from commit 9326a08)
(cherry picked from commit 6eac7e5)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 27, 2024
…t channel (apache#22859)

(cherry picked from commit 9326a08)
(cherry picked from commit 6eac7e5)
(cherry picked from commit 9ffbffc)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 1, 2024
…t channel (apache#22859)

(cherry picked from commit 9326a08)
(cherry picked from commit 6eac7e5)
(cherry picked from commit 9ffbffc)
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants