jbewing commented on code in PR #5373:
URL: https://github.com/apache/hbase/pull/5373#discussion_r1385442557


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java:
##########
@@ -486,52 +500,171 @@ public boolean shouldUseScanner(Scan scan, HStore store, 
long oldestUnexpiredTS)
   @Override
   public boolean seekToPreviousRow(Cell originalKey) throws IOException {
     try {
-      try {
-        boolean keepSeeking = false;
-        Cell key = originalKey;
-        do {
-          Cell seekKey = PrivateCellUtil.createFirstOnRow(key);
-          if (seekCount != null) seekCount.increment();
-          if (!hfs.seekBefore(seekKey)) {
-            this.cur = null;
-            return false;
-          }
-          Cell curCell = hfs.getCell();
-          Cell firstKeyOfPreviousRow = 
PrivateCellUtil.createFirstOnRow(curCell);
-
-          if (seekCount != null) seekCount.increment();
-          if (!seekAtOrAfter(hfs, firstKeyOfPreviousRow)) {
-            this.cur = null;
-            return false;
-          }
-
-          setCurrentCell(hfs.getCell());
-          this.stopSkippingKVsIfNextRow = true;
-          boolean resultOfSkipKVs;
-          try {
-            resultOfSkipKVs = skipKVsNewerThanReadpoint();
-          } finally {
-            this.stopSkippingKVsIfNextRow = false;
-          }
-          if (!resultOfSkipKVs || getComparator().compareRows(cur, 
firstKeyOfPreviousRow) > 0) {
-            keepSeeking = true;
-            key = firstKeyOfPreviousRow;
-            continue;
-          } else {
-            keepSeeking = false;
-          }
-        } while (keepSeeking);
-        return true;
-      } finally {
-        realSeekDone = true;
+      if (isFastSeekingEncoding) {
+        return seekToPreviousRowStateless(originalKey);
+      } else if (previousRow == null || 
getComparator().compareRows(previousRow, originalKey) > 0) {
+        return seekToPreviousRowWithoutHint(originalKey);
+      } else {
+        return seekToPreviousRowWithHint(originalKey);
       }
     } catch (FileNotFoundException e) {
       throw e;
     } catch (IOException ioe) {
       throw new IOException("Could not seekToPreviousRow " + this + " to key " 
+ originalKey, ioe);
+    } finally {
+      this.realSeekDone = true;
     }
   }
 
+  private boolean seekToPreviousRowWithHint(Cell originalKey) throws 
IOException {
+    do {
+      if (previousRow == null) {
+        return seekToPreviousRowWithoutHint(originalKey);
+      }
+
+      Cell firstKeyOfPreviousRow = 
PrivateCellUtil.createFirstOnRow(previousRow);
+      if (!seekBeforeAndSaveKeyToPreviousRow(firstKeyOfPreviousRow)) {
+        return false;
+      }
+
+      if (!reseekAtOrAfter(firstKeyOfPreviousRow)) {
+        return false;
+      }
+
+      if (
+        setReadpointAndSkipNewerKvs()
+          && getComparator().compareRows(cur, firstKeyOfPreviousRow) <= 0
+      ) {
+        return true;
+      }
+    } while (true);
+  }
+
+  private boolean seekToPreviousRowWithoutHint(Cell originalKey) throws 
IOException {
+    boolean keepSeeking;
+    Cell key = originalKey;
+    do {
+      // Rewind to the cell before the beginning of this row
+      Cell keyAtBeginningOfRow = PrivateCellUtil.createFirstOnRow(key);
+      if (!seekBefore(keyAtBeginningOfRow)) {
+        return false;
+      }
+
+      // Rewind before this row and save what we find as a seek hint
+      Cell firstKeyOfPreviousRow = 
PrivateCellUtil.createFirstOnRow(hfs.getCell());
+      if (!seekBeforeAndSaveKeyToPreviousRow(firstKeyOfPreviousRow)) {
+        return false;
+      }
+
+      // Seek back to the start of the previous row
+      if (!reseekAtOrAfter(firstKeyOfPreviousRow)) {
+        return false;
+      }
+
+      if (
+        !setReadpointAndSkipNewerKvs()
+          || getComparator().compareRows(cur, firstKeyOfPreviousRow) > 0
+      ) {
+        keepSeeking = true;
+        key = firstKeyOfPreviousRow;
+      } else {
+        keepSeeking = false;

Review Comment:
   Incorporated this by introducing a new helper method
   ```java
   private boolean isStillAtSeekTargetAfterSkippingNewerKvs(Cell seekKey) 
throws IOException {
       setCurrentCell(hfs.getCell());
       return skipNewerKvsNewerThanReadpointReversed() && 
getComparator().compareRows(cur, seekKey) <= 0;
     }
   
   private boolean skipNewerKvsNewerThanReadpointReversed() throws IOException {
       this.stopSkippingKVsIfNextRow = true;
       boolean resultOfSkipKVs;
       try {
         resultOfSkipKVs = skipKVsNewerThanReadpoint();
       } finally {
         this.stopSkippingKVsIfNextRow = false;
       }
   
       return resultOfSkipKVs;
     }
   ```
   
   that are then utilized like this:
   ```java
   if (isStillAtSeekTargetAfterSkippingNewerKvs(firstKeyOfPreviousRow)) {
      return true;
   }
   key = firstKeyOfPreviousRow;
   ```



-- 
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

Reply via email to