gjacoby126 commented on a change in pull request #592: PHOENIX-5505 Index read
repair does not repair unverified rows with h…
URL: https://github.com/apache/phoenix/pull/592#discussion_r330808776
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java
##########
@@ -247,9 +255,12 @@ private void repairIndexRows(byte[] indexRowKey, long ts,
List<Cell> row) throws
buildIndexScan.setAttribute(BaseScannerRegionObserver.SKIP_REGION_BOUNDARY_CHECK,
Bytes.toBytes(true));
}
// Rebuild the index rows from the corresponding the rows in the
the data table
+ // Get the data row key from the index row key
byte[] dataRowKey = indexMaintainer.buildDataRowKey(new
ImmutableBytesWritable(indexRowKey), viewConstants);
+ // Rebuild the index rows from the data rows starting with the
data row key
buildIndexScan.withStartRow(dataRowKey, true);
- buildIndexScan.setTimeRange(ts, maxTimestamp);
+ buildIndexScan.setTimeRange(0, maxTimestamp);
Review comment:
I'm a bit confused by the startTime here. I'd have expected it to be either
{0, Long.MAX_VALUE} to look at all rows, {0,
EnvironmentEdgeManager.currentTimeMillis()} to look up to the current server
timestamp, or {Math.max(0, minTimestamp), Math.min(maxTimestamp,
Long.MAX_VALUE}, to look at the timestamps visible to the user Scan, where
minTimestamp and maxTimestamp come from, but not a mixture of 0 and
maxTimestamp.
If we do honor the minTimestamp, we need to make sure not to delete from the
index based on not finding a data row that might be hidden by the minTimestamp.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
