sanjeet006py commented on code in PR #2574:
URL: https://github.com/apache/phoenix/pull/2574#discussion_r3709947315
##########
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:
Oh, I didn't relaize that if its not going through RPC path then it won't
even consume additional handler threads. Yeah, there is no benefit of paging in
here. Thanks, will remove it.
##########
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:
Oh, I didn't realize that if its not going through RPC path then it won't
even consume additional handler threads. Yeah, there is no benefit of paging in
here. Thanks, will remove it.
--
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]
