BewareMyPower opened a new pull request #12953:
URL: https://github.com/apache/pulsar/pull/12953
### Motivation
The `ConcurrentOpenLongPairRangeSet#isEmpty` implementation is wrong. See
```java
AtomicBoolean isEmpty = new AtomicBoolean(false);
rangeBitSetMap.forEach((key, val) -> {
if (!isEmpty.get()) { // [1] isEmpty is always false, so this
condition always meets
return;
isEmpty.set(val.isEmpty()); // it never reaches here
});
return isEmpty.get(); // [2] So if rangeBitSetMap is not empty,
isEmpty() always return true
```
The comment of `[1]` should be `isEmpty.get()`, not `!isEmpty.get()`.
However, the implementation is still bad because `forEach` method will iterate
over the whole map. If the map has many entries, the performance cost will not
be ignorable.
### Modifications
- Fix `ConcurrentOpenLongPairRangeSet#isEmpty`. Instead of calling
`forEach`, here we use a trivial for loop and break the loop if there is an
entry whose value is a non-empty set.
- Add a test (`testIsEmpty`), which adds an empty set to
`ConcurrentOpenLongPairRangeSet`. Before this patch, the `isEmpty()` returns
false, which is incorrect.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added test `ConcurrentOpenLongPairRangeSetTest#testIsEmpty`.
--
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]
