Improve getManagedLedgerOffloader equality check for OffloadPoliciesImpl
- Dominant language
- Java
- Stars
- 15.3k
- Forks
- 3.8k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 160
Description
### Search before asking
- [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.
### Motivation
The equality check on the following line does not return true based on my testing from a couple years ago :) (I meant to open this issue a while ago, but I just found the reference today, and since the less than optimal code is present, now is as good a time as ever.)
https://github.com/apache/pulsar/blob/82feb13949e9387098e93d0484e1c0f622b7e0d5/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java#L1346
The problem comes in how the two `offloadPolicies` are instantiated. Reading through the code may reveal the issue. I discovered it by running through the code with a debugger. Then, I noticed that certain fields evaluate to `null` while others are `""` depending on how they the policy object is initialized.
### Solution
* Fix how the objects are initialized.
* Also, we might want to explore if the `ledgerOffloaderMap` has a leak of `offloaders`. I don't see any interactions that remove entries from the `ledgerOffloaderMap` map.
### Alternatives
Fix how the object equality is checked. This could have unintended consequences. Correcting how the objects are deserialized seems like the better approach to me.
### Anything else?
I will try to submit a PR, but I might not have time.
### Are you willing to submit a PR?
- [ ] I'm willing to submit a PR!
Contributor guide
Research direction
Start at the equality check in pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java around line 1346 and trace how the two OffloadPoliciesImpl objects are initialized or deserialized. Compare the null and empty-string fields observed in the issue, then inspect ledgerOffloaderMap for entries that are never removed. Done means the policy comparison behaves correctly and any confirmed offloader lifecycle issue is addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100