junegunn commented on code in PR #8001:
URL: https://github.com/apache/hbase/pull/8001#discussion_r3036328156


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NormalUserScanQueryMatcher.java:
##########
@@ -70,9 +95,31 @@ public MatchCode match(ExtendedCell cell) throws IOException 
{
         seePastDeleteMarkers ? tr.withinTimeRange(timestamp) : 
tr.withinOrAfterTimeRange(timestamp);
       if (includeDeleteMarker) {
         this.deletes.add(cell);
+        // A DeleteColumn or DeleteFamily masks all remaining cells for this 
column/family.
+        // Seek past them instead of skipping one cell at a time, but only 
after seeing
+        // enough consecutive markers for the same column to justify the seek 
overhead.
+        // Only safe with plain ScanDeleteTracker. Not safe with 
newVersionBehavior (sequence
+        // IDs determine visibility), visibility labels (delete/put label 
mismatch), or
+        // seePastDeleteMarkers (KEEP_DELETED_CELLS).
+        if (
+          canSeekOnDeleteMarker && (typeByte == 
KeyValue.Type.DeleteFamily.getCode()
+            || (typeByte == KeyValue.Type.DeleteColumn.getCode() && 
cell.getQualifierLength() > 0))
+        ) {
+          if (lastDelete != null && !CellUtil.matchingQualifier(cell, 
lastDelete)) {
+            rangeDeleteCount = 0;
+          }
+          lastDelete = cell;
+          if (++rangeDeleteCount >= SEEK_ON_DELETE_MARKER_THRESHOLD) {
+            rangeDeleteCount = 0;
+            return columns.getNextRowOrNextColumn(cell);
+          }
+        } else {
+          rangeDeleteCount = 0;
+        }

Review Comment:
   So here it is:
   
   ```ruby
   # The minimum timestamp of GET and SCAN will be set to this value
   future = (Time.now.to_i + 3600) * 1000
   benchmark(:DeleteColumnOldTimestamps, future) do |i|
     T.put(Put.new(ROW).addColumn(CF, CQ, future, VALUE)) if i.zero?
     T.delete(DC)
     flush 't' if (i % 100_000).zero? && i.positive?
   end
   ```
   
   - This benchmark code will `setTimeRange(future, MAX)` both for GET and SCAN
   - If we don't put an actual cell within the range, the scanner will be 
skipped entirely, so we "put" with the future timestamp
   
   This is a very contrived scenario, to be honest, but anyway, moving the 
block out of `includeDeleteMarker` condition (`HBASE-29039-alt-n10-mod`) helps 
a lot in this case.
   
   <img width="2304" height="1920" alt="image" 
src="https://github.com/user-attachments/assets/ba5782c2-19ff-4967-8db7-31ee7335d8fa";
 />
   
   I updated the code and added a test case in 5b8afdc0fe.



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

Reply via email to