bbeaudreault commented on code in PR #5373:
URL: https://github.com/apache/hbase/pull/5373#discussion_r1383356199
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java:
##########
@@ -485,6 +499,124 @@ public boolean shouldUseScanner(Scan scan, HStore store,
long oldestUnexpiredTS)
@Override
public boolean seekToPreviousRow(Cell originalKey) throws IOException {
+ if (isFastSeekingEncoding) {
+ return seekToPreviousRowStateless(originalKey);
Review Comment:
I think we need to DRY this up. Previously, seekToPreviousRow already
required careful reading to see what's happening. Now we have 3 versions of
that method, all very similar but with subtle differences. AFAICT they all have
the exact same beginning and end, with slight differences in the middle? Maybe
we can pull it out to a small abstract inner class with 3 impls? Or maybe we
can pass in a Function or other state to the sub-methods to trigger the correct
behavior?
While here, I think this method would read nicer if you broke out the 3rd
impl below into its own method:
```java
public boolean seekToPreviousRow(Cell originalKey) throws IOException {
if (isFastSeekingEncoding) {
return seekToPreviousRowStateless(originalKey);
} else if (previousRow == null || getComparator().compareRows(previousRow,
originalKey) > 0) {
return seekToPreviousRowWithoutHint(originalKey);
} else {
return seekToPreviousRowWithHint(originalKey, previousRow);
}
}
```
--
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]
