virajjasani commented on code in PR #1736:
URL: https://github.com/apache/phoenix/pull/1736#discussion_r1456518028
##########
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/OffsetResultIterator.java:
##########
@@ -32,32 +34,49 @@
*/
public class OffsetResultIterator extends DelegateResultIterator {
private int rowCount;
- private int offset;
+ private final int offset;
+ private Tuple lastScannedTuple;
private long pageSizeMs = Long.MAX_VALUE;
+ private boolean isIncompatibleClient = false;
public OffsetResultIterator(ResultIterator delegate, Integer offset) {
super(delegate);
this.offset = offset == null ? -1 : offset;
+ this.lastScannedTuple = null;
}
- public OffsetResultIterator(ResultIterator delegate, Integer offset, long
pageSizeMs) {
+ public OffsetResultIterator(ResultIterator delegate, Integer offset, long
pageSizeMs,
+ boolean isIncompatibleClient) {
this(delegate, offset);
this.pageSizeMs = pageSizeMs;
+ this.isIncompatibleClient = isIncompatibleClient;
}
+
@Override
public Tuple next() throws SQLException {
+ long startTime = EnvironmentEdgeManager.currentTimeMillis();
while (rowCount < offset) {
Tuple tuple = super.next();
- if (tuple == null) { return null; }
+ if (tuple == null) {
+ return null;
+ }
if (tuple.size() == 0 || isDummy(tuple)) {
// while rowCount < offset absorb the dummy and call next on
the underlying scanner
continue;
}
rowCount++;
- // no page timeout check at this level because we cannot correctly
resume
- // scans for OFFSET queries until the offset is reached
+ lastScannedTuple = tuple;
+ if (!isIncompatibleClient) {
+ if (EnvironmentEdgeManager.currentTimeMillis() - startTime >=
pageSizeMs) {
Review Comment:
I am not able to reproduce any issue with this.
OffsetScanner will return valid tuple with cell CF:CQ as
`f_offset:c_offset`, from which the cell value will be used by SerialIterators
to set offset on next scan:
```
while (index < scans.size()) {
Scan currentScan = scans.get(index++);
if (remainingOffset != null) {
// <======= set offset value
currentScan.setAttribute(BaseScannerRegionObserverConstants.SCAN_OFFSET,
PInteger.INSTANCE.toBytes(remainingOffset));
}
ScanMetricsHolder scanMetricsHolder =
ScanMetricsHolder.getInstance(readMetrics,
tableName, currentScan,
context.getConnection().getLogLevel());
TableResultIterator itr =
new TableResultIterator(mutationState, currentScan,
scanMetricsHolder,
renewLeaseThreshold, plan, scanGrouper,
caches, maxQueryEndTime);
PeekingResultIterator peekingItr =
iteratorFactory.newIterator(context, itr, currentScan, tableName, plan);
Tuple tuple;
if ((tuple = peekingItr.peek()) == null) {
peekingItr.close();
continue;
// <======== retrieve offset value from server
} else if ((remainingOffset =
QueryUtil.getRemainingOffset(tuple)) != null) {
peekingItr.next();
peekingItr.close();
continue;
}
context.getConnection().addIteratorForLeaseRenewal(itr);
return peekingItr;
}
```
Hence, depending on the cell returned by server, next scan's offset
attribute will be set and accordingly new scanner will take that value.
Basically, i will have to remove page timeout logic from offset because of
it's tricky to manage returning dummy, and if we do that, we will anyways
ensure that server is able to return valid offset value to client and client
will check whether more serial iteration needs to happen depending on how many
rows were scanned.
The logic remains same even if region moves after returning valid row. The
problem happens only when region moves after dummy is returned, so we will
remove page timeout for offset scanner.
--
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]
