eolivelli commented on a change in pull request #10480:
URL: https://github.com/apache/pulsar/pull/10480#discussion_r625751106
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/util/RangeCache.java
##########
@@ -90,7 +90,7 @@ public Value get(Key key) {
try {
value.retain();
return value;
- } catch (Throwable t) {
+ } catch (IllegalReferenceCountException e) {
Review comment:
probably we should log something here.
this case must not happen
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/util/RangeCache.java
##########
@@ -113,7 +113,7 @@ public Value get(Key key) {
try {
value.retain();
values.add(value);
- } catch (Throwable t) {
+ } catch (IllegalReferenceCountException e) {
Review comment:
probably we should log something here.
this case must not happen
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/util/RangeCache.java
##########
@@ -168,9 +170,11 @@ public Value get(Key key) {
}
Value value = entry.getValue();
- ++removedEntries;
- removedSize += weighter.getSize(value);
- value.release();
+ long entrySize = weighter.getSize(value);
+ if (value.invalidate()) {
Review comment:
I am afraid we are only hiding the problem.
there must be some coordination about these entries, some clear protocol
about who is the owner of the entry.
when we get to this point then we must be sure that the refcount is valid on
the value, otherwise it is always an hazard.
I believe that the right protocol is that `before` calling`
weighter.getSize(value);` we should `try` to acquire the entry and in case of
failure we can ignore the entry.
```
Value value = entry.getValue();
if (value.tryAcquire()) {
++removedEntries;
removedSize += weighter.getSize(value);
value.release(); // this refers to the value.tryAcquire()
}
```
when we remove the entry we must have some write lock over the entry, that
prevents double releases
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
