keith-turner commented on code in PR #3644:
URL: https://github.com/apache/accumulo/pull/3644#discussion_r1273792470
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Scanner.java:
##########
@@ -162,22 +162,38 @@ public ScanBatch read() throws IOException,
TabletClosedException {
tablet.updateQueryStats(results.getResults().size(),
results.getNumBytes());
}
} finally {
- scannerSemaphore.release();
+ lock.unlock();
Review Comment:
```suggestion
inRead=false;
lock.unlock();
```
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Scanner.java:
##########
@@ -72,7 +72,7 @@ public ScanBatch read() throws IOException,
TabletClosedException {
try {
try {
- scannerSemaphore.acquire();
+ lock.lockInterruptibly();
Review Comment:
I think part of what the semaphore was doing was preventing recursive calls
to this function. I don't the the current code ever does this, but if it ever
did in the future it would leave the instance variable in this class in a bad
state. I think a boolean along with the lock can preserve this behavior.
```suggestion
lock.lockInterruptibly();
Preconditions.checkState(!inRead);
// Simple check to ensure the same thread never calls this method
recursively. This code would not handle that well.
inRead = true;
```
--
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]
