swaroopak commented on a change in pull request #672: PHOENIX-5658 IndexTool to
verify index rows inline
URL: https://github.com/apache/phoenix/pull/672#discussion_r363909941
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
##########
@@ -1189,40 +1237,168 @@ private Delete generateDeleteMarkers(List<Cell> row) {
checkForRegionClosing();
commitBatchWithRetries(region, mutations,
blockingMemstoreSize);
uuidValue = ServerCacheClient.generateId();
+ if (verify) {
+ addToBeVerifiedIndexRows();
+ }
mutations.clear();
}
return uuidValue;
}
- private boolean checkIndexRow(final byte[] indexRowKey, final Put put)
throws IOException {
- ValueGetter getter = new ValueGetter() {
- final ImmutableBytesWritable valuePtr = new
ImmutableBytesWritable();
-
- @Override
- public ImmutableBytesWritable getLatestValue(ColumnReference
ref, long ts) throws IOException {
- List<Cell> cellList = put.get(ref.getFamily(),
ref.getQualifier());
- if (cellList == null || cellList.isEmpty()) {
- return null;
- }
- Cell cell = cellList.get(0);
- valuePtr.set(cell.getValueArray(), cell.getValueOffset(),
cell.getValueLength());
- return valuePtr;
+ private class SimpleValueGetter implements ValueGetter {
+ final ImmutableBytesWritable valuePtr = new
ImmutableBytesWritable();
+ final Put put;
+ SimpleValueGetter (final Put put) {
+ this.put = put;
+ }
+ @Override
+ public ImmutableBytesWritable getLatestValue(ColumnReference ref,
long ts) throws IOException {
+ List<Cell> cellList = put.get(ref.getFamily(),
ref.getQualifier());
+ if (cellList == null || cellList.isEmpty()) {
+ return null;
}
+ Cell cell = cellList.get(0);
+ valuePtr.set(cell.getValueArray(), cell.getValueOffset(),
cell.getValueLength());
+ return valuePtr;
+ }
- @Override
- public byte[] getRowKey() {
- return put.getRow();
- }
- };
- byte[] builtIndexRowKey = indexMaintainer.buildRowKey(getter, new
ImmutableBytesWritable(put.getRow()),
+ @Override
+ public byte[] getRowKey() {
+ return put.getRow();
+ }
+
+ }
+
+ private byte[] getIndexRowKey(final Put dataRow) throws IOException {
+ ValueGetter valueGetter = new SimpleValueGetter(dataRow);
+ byte[] builtIndexRowKey = indexMaintainer.buildRowKey(valueGetter,
new ImmutableBytesWritable(dataRow.getRow()),
null, null, HConstants.LATEST_TIMESTAMP);
+ return builtIndexRowKey;
+ }
+
+ private boolean checkIndexRow(final byte[] indexRowKey, final Put put)
throws IOException {
+ byte[] builtIndexRowKey = getIndexRowKey(put);
if (Bytes.compareTo(builtIndexRowKey, 0, builtIndexRowKey.length,
indexRowKey, 0, indexRowKey.length) != 0) {
return false;
}
return true;
}
+ private void verifySingleIndexRow(Result indexRow, final Put dataRow)
throws IOException {
+ ValueGetter valueGetter = new SimpleValueGetter(dataRow);
+ long ts = 0;
+ for (List<Cell> cells : dataRow.getFamilyCellMap().values()) {
+ if (cells == null) {
+ break;
+ }
+ for (Cell cell : cells) {
+ if (ts < cell.getTimestamp()) {
+ ts = cell.getTimestamp();
+ }
+ }
+ }
+ Put indexPut = indexMaintainer.buildUpdateMutation(kvBuilder,
valueGetter, new ImmutableBytesWritable(dataRow.getRow()), ts, null, null);
+ if (indexPut == null) {
+ // This means the index row does not have any covered columns.
We just need to check if the index row
+ // has only one cell (which is the empty column cell)
+ if (indexRow.rawCells().length == 1) {
+ return;
+ }
+ exceptionMessage = "Index verify failed - Expected to find
only empty column cell but got "
+ + indexRow.rawCells().length + " cells - " +
indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ else {
+ // Remove the empty column prepared by Index codec as we need
to change its value
+ removeEmptyColumn(indexPut,
indexMaintainer.getEmptyKeyValueFamily().copyBytesIfNecessary(),
+ indexMaintainer.getEmptyKeyValueQualifier());
+ }
+ // Add the empty column
+
indexPut.addColumn(indexMaintainer.getEmptyKeyValueFamily().copyBytesIfNecessary(),
+ indexMaintainer.getEmptyKeyValueQualifier(), ts,
VERIFIED_BYTES);
+ int cellCount = 0;
+ for (List<Cell> cells : indexPut.getFamilyCellMap().values()) {
+ if (cells == null) {
+ break;
+ }
+ for (Cell expectedCell : cells) {
+ Cell actualCell =
indexRow.getColumnLatestCell(CellUtil.cloneFamily(expectedCell),
+ CellUtil.cloneQualifier(expectedCell));
+ if (actualCell == null) {
+ exceptionMessage = "Index verify failed - Missing cell
" + indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ // Check all columns
+ if (!CellUtil.matchingValue(actualCell, expectedCell)) {
+ exceptionMessage = "Index verify failed - Not matching
cell value - " + indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ cellCount++;
+ }
+ }
+ if (cellCount != indexRow.rawCells().length) {
+ exceptionMessage = "Index verify failed - Expected to find " +
cellCount + " cells but got "
+ + indexRow.rawCells().length + " cells - " +
indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ }
+
+ private void verifyIndexRows(ArrayList<KeyRange> keys) throws
IOException {
+ int expectedRowCount = keys.size();
+ ScanRanges scanRanges = ScanRanges.createPointLookup(keys);
+ Scan indexScan = new Scan();
+ indexScan.setTimeRange(scan.getTimeRange().getMin(),
scan.getTimeRange().getMax());
+ scanRanges.initializeScan(indexScan);
+ SkipScanFilter skipScanFilter = scanRanges.getSkipScanFilter();
+ indexScan.setFilter(skipScanFilter);
+ int rowCount = 0;
+ try (ResultScanner resultScanner =
indexHTable.getScanner(indexScan)) {
+ for (Result result = resultScanner.next(); (result != null);
result = resultScanner.next()) {
+ Put dataPut = dataPutMap.get(result.getRow());
+ if (dataPut == null) {
+ exceptionMessage = "Index verify failed - Missing data
row - " + indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ verifySingleIndexRow(result, dataPut);
+ rowCount++;
+ }
+ } catch (Throwable t) {
+ ServerUtil.throwIOException(indexHTable.getName().toString(),
t);
+ }
+ if (rowCount != expectedRowCount) {
+ exceptionMessage = "Index verify failed - Missing index rows
Expected : " + expectedRowCount +
+ " Actual : " + rowCount + " - " +
indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ }
+
+ private void addVerifyTask(final ArrayList<KeyRange> keys) {
+ tasks.add(new Task<Boolean>() {
+ @Override
+ public Boolean call() throws Exception {
+ try {
+ if (Thread.currentThread().isInterrupted()) {
+ exceptionMessage = "Pool closed, not attempting to
verify index rows! " + indexHTable.getName();
+ throw new IOException(exceptionMessage);
+ }
+ verifyIndexRows(keys);
+ } catch (Exception e) {
+ throw e;
+ }
+ return Boolean.TRUE;
+ }
+ });
+ }
+
+ private void addToBeVerifiedIndexRows() throws IOException {
Review comment:
nit: do you think moving this method close to the caller makes it more
readable?
----------------------------------------------------------------
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
