This is an automated email from the ASF dual-hosted git repository. kadir pushed a commit to branch 4.x-HBase-1.5 in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x-HBase-1.5 by this push: new 0c52cf7 PHOENIX-5615 Index read repair should delete all the cells of an invalid unverified row 0c52cf7 is described below commit 0c52cf79256bd096195758c9a867f4bce7d732f2 Author: Kadir <kozde...@salesforce.com> AuthorDate: Thu Dec 12 05:00:13 2019 -0800 PHOENIX-5615 Index read repair should delete all the cells of an invalid unverified row --- .../phoenix/end2end/index/GlobalIndexCheckerIT.java | 18 +++++++++++++++--- .../org/apache/phoenix/index/GlobalIndexChecker.java | 13 ++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexCheckerIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexCheckerIT.java index 9a86bb4..5861323 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexCheckerIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/GlobalIndexCheckerIT.java @@ -37,6 +37,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.phoenix.end2end.BaseUniqueNamesOwnClusterIT; import org.apache.phoenix.end2end.IndexToolIT; import org.apache.phoenix.hbase.index.IndexRegionObserver; +import org.apache.phoenix.query.QueryServices; import org.apache.phoenix.util.PhoenixRuntime; import org.apache.phoenix.util.QueryUtil; import org.apache.phoenix.util.ReadOnlyProps; @@ -66,6 +67,7 @@ public class GlobalIndexCheckerIT extends BaseUniqueNamesOwnClusterIT { @BeforeClass public static synchronized void doSetup() throws Exception { Map<String, String> props = Maps.newHashMapWithExpectedSize(1); + props.put(QueryServices.GLOBAL_INDEX_ROW_AGE_THRESHOLD_TO_DELETE_MS_ATTRIB, Long.toString(0)); setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator())); } @@ -318,18 +320,28 @@ public class GlobalIndexCheckerIT extends BaseUniqueNamesOwnClusterIT { // update phase) and check that this does not impact the correctness (one overwrite) IndexRegionObserver.setFailDataTableUpdatesForTesting(true); IndexRegionObserver.setFailPostIndexUpdatesForTesting(true); - conn.createStatement().execute("upsert into " + dataTableName + " (id, val2) values ('a', 'abcc')"); + conn.createStatement().execute("upsert into " + dataTableName + " (id, val2, val3) values ('a', 'abcc', 'abccc')"); commitWithException(conn); IndexRegionObserver.setFailDataTableUpdatesForTesting(false); IndexRegionObserver.setFailPostIndexUpdatesForTesting(false); - String selectSql = "SELECT val2, val3 from " + dataTableName + " WHERE val1 = 'ab'"; + // Read only one column and verify that this is sufficient for the read repair to fix + // all the columns of the unverified index row that was generated due to doing only one phase write above + String selectSql = "SELECT val2 from " + dataTableName + " WHERE val1 = 'ab'"; // Verify that we will read from the first index table assertExplainPlan(conn, selectSql, dataTableName, indexTableName + "1"); // Verify that one phase write has no effect ResultSet rs = conn.createStatement().executeQuery(selectSql); assertTrue(rs.next()); assertEquals("abc", rs.getString(1)); - assertEquals("abcd", rs.getString(2)); + assertFalse(rs.next()); + // Now read the other column and verify that it is also fixed + selectSql = "SELECT val3 from " + dataTableName + " WHERE val1 = 'ab'"; + // Verify that we will read from the first index table + assertExplainPlan(conn, selectSql, dataTableName, indexTableName + "1"); + // Verify that one phase write has no effect + rs = conn.createStatement().executeQuery(selectSql); + assertTrue(rs.next()); + assertEquals("abcd", rs.getString(1)); assertFalse(rs.next()); selectSql = "SELECT val2, val3 from " + dataTableName + " WHERE val2 = 'abcc'"; // Verify that we will read from the second index table diff --git a/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java b/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java index b9ccae8..c7f79ae 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java @@ -114,10 +114,12 @@ public class GlobalIndexChecker extends BaseRegionObserver { * and used to verify individual rows and rebuild them if they are not valid */ private class GlobalIndexScanner implements RegionScanner { - RegionScanner scanner; + private RegionScanner scanner; + private RegionScanner deleteRowScanner; private long ageThreshold; private Scan scan; private Scan indexScan; + private Scan deleteRowScan; private Scan singleRowIndexScan; private Scan buildIndexScan = null; private Table dataHTable = null; @@ -246,6 +248,14 @@ public class GlobalIndexChecker extends BaseRegionObserver { if ((EnvironmentEdgeManager.currentTimeMillis() - ts) > ageThreshold) { Delete del = new Delete(indexRowKey, ts); if (specific) { + // Get all the cells of this row + deleteRowScan.withStartRow(indexRowKey, true); + deleteRowScan.withStopRow(indexRowKey, true); + deleteRowScan.setTimeRange(0, ts + 1); + deleteRowScanner = region.getScanner(deleteRowScan); + row.clear(); + deleteRowScanner.next(row); + deleteRowScanner.close(); // We are deleting a specific version of a row so the flowing loop is for that for (Cell cell : row) { del.addColumn(CellUtil.cloneFamily(cell), CellUtil.cloneQualifier(cell), cell.getTimestamp()); @@ -261,6 +271,7 @@ public class GlobalIndexChecker extends BaseRegionObserver { if (buildIndexScan == null) { buildIndexScan = new Scan(); indexScan = new Scan(scan); + deleteRowScan = new Scan(); singleRowIndexScan = new Scan(scan); byte[] dataTableName = scan.getAttribute(PHYSICAL_DATA_TABLE_NAME); byte[] indexTableName = region.getRegionInfo().getTable().getName();