merlimat commented on a change in pull request #6787:
URL: https://github.com/apache/pulsar/pull/6787#discussion_r413133123
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainer.java
##########
@@ -164,6 +175,22 @@ public ManagedCursor getSlowestReader() {
}
}
+ public boolean hasCursors() {
Review comment:
It's becoming a bit confusing now to understand the meaning of
`hasCursors()` and `isEmpty()`. We should be use a more unambiguous/explicit
naming here.
My suggestion:
* `hasCursors()` --> `isEmtpy()` // Since we're talking about a cursors
container
* `isEmpty()` --> `hasDurableCursors()`
(and fix the reversal of the boolean logic)
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorContainer.java
##########
@@ -164,6 +175,22 @@ public ManagedCursor getSlowestReader() {
}
}
+ public boolean hasCursors() {
+ long stamp = rwLock.tryOptimisticRead();
+ boolean isEmpty = cursors.isEmpty();
+ if (!rwLock.validate(stamp)) {
+ // Fallback to read lock
+ stamp = rwLock.readLock();
+ try {
+ isEmpty = cursors.isEmpty();
Review comment:
Wouldn't the logic be the reverse here? We're checking `isEmpty` but the
method should return true if it's not empty, no?
----------------------------------------------------------------
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]
