bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073566824
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:
##########
@@ -612,6 +612,9 @@ public boolean next(List<Cell> outResult, ScannerContext
scannerContext) throws
// here, we still need to scan all the qualifiers before returning...
scannerContext.returnImmediately();
}
+
+ scannerContext.incrementBlockProgress(heap.getCurrentBlockSizeOnce());
Review Comment:
@Apache9 here's a commit which implements the `getBlockChanged()` approach:
https://github.com/HubSpot/hbase/commit/c619b4f3f7cd7f9e9677b265e10f0f10edefcf23
Let me know if you'd like me to push that here instead. It's better in some
ways, but still has one unfortunate caveat described in the javadoc of
KeyValueScanner. See the "Note:" portion here:
```java
/**
* Returns true if the current block has changed since last invocation.
Subsequent calls to this
* method will return false, until the block changes again. <br>
* Note: This value is propagated up from the HFile.Reader. The value of
this result is only valid
* until the next call to {@link #next()} or any other seek-related
method. Calling those methods
* may change which underlying HFile is currently being read, which will
reset the block changed
* state.
*/
```
This is ok for our current use-case, since we always invoke the 2 methods
together. But in some ways it would be better to combine these 2 methods into
one (like in `getCurrentBlockSizeOnce()`) so that you can't stumble across that
note.
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
