junegunn commented on code in PR #8001:
URL: https://github.com/apache/hbase/pull/8001#discussion_r3036225452
##########
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:
Good point! I felt it was easier to reason about the code when it's inside
this block, but on second thought, it should be safe to move it out of it, and
it might provide performance benefit in some very rare cases. No cons I can
think of. Let me see if I can come up with a contrived benchmark scenario to
show the difference.
--
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]