sanjeet006py commented on code in PR #2574:
URL: https://github.com/apache/phoenix/pull/2574#discussion_r3709916965
##########
phoenix-core-server/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java:
##########
@@ -1228,20 +1270,41 @@ private void
getCurrentRowStates(ObserverContext<RegionCoprocessorEnvironment> c
scanRanges.initializeScan(scan);
SkipScanFilter skipScanFilter = scanRanges.getSkipScanFilter();
scan.setFilter(skipScanFilter);
+ // Anchor TTLRegionScanner's masking clock at batchTimestamp. The range
upper bound is
+ // batchTimestamp + 1 (not batchTimestamp) because HBase scan time
ranges are half-open
+ // [min, max): a bound of batchTimestamp would exclude any committed
cell sitting at exactly
+ // batchTimestamp. That collision cannot happen in production
(getBatchTimestamp forces a
+ // distinct timestamp for same-row batches), but a frozen test clock can
produce it, and the
+ // read must see those cells to build a correct current-row state. The
masking clock is then
+ // batchTimestamp + 1, trimming expired cells as of the timestamp the
index is built at.
+ scan.setTimeRange(0, batchTimestamp + 1);
+
ServerScanUtil.setInternalScanAttributes(c.getEnvironment().getConfiguration(),
scan, emptyCF,
+ emptyCQ, literalTTLForScan, isStrictTTL);
readDataTableRows(c, context, scan);
}
}
private void readDataTableRows(ObserverContext<RegionCoprocessorEnvironment>
c,
BatchMutateContext context, Scan scan) throws IOException {
- try (RegionScanner scanner =
c.getEnvironment().getRegion().getScanner(scan)) {
+ // Open through ServerScanUtil so the scan is wrapped in TTLRegionScanner
(mirroring
+ // postScannerOpen) and masks TTL-expired rows exactly like a client read.
Masking no-ops when
+ // Phoenix compaction is off or the empty-column attributes are absent.
+ try (RegionScanner scanner =
+ ServerScanUtil.openRegionScanner(c.getEnvironment(),
c.getEnvironment().getRegion(), scan)) {
boolean more = true;
while (more) {
List<Cell> cells = new ArrayList<Cell>();
more = scanner.next(cells);
if (cells.isEmpty()) {
continue;
}
+ // With server paging wired in
(ServerScanUtil.setInternalScanAttributes),
+ // PagingRegionScanner
+ // returns a dummy result when a page is paged out; skip it and let
the loop resume rather
+ // than build a Put from the dummy cell.
+ if (ScanUtil.isDummy(cells)) {
+ continue;
+ }
Review Comment:
There is no explicit functional gain rather this is scan path and my
understanding is we want to cover all scan paths via paging to ensure fair
utilization of server resources/handler threads. Thus, added paging explicitly.
This way internal scan done by IndexRegionObserver by directly opening region
scanner won't bypass paging.
--
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]
