You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ka...@apache.org on 2020/03/30 16:53:43 UTC
[phoenix] branch master updated: PHOENIX-5807 Index rows without
empty column should be treated as unverified
This is an automated email from the ASF dual-hosted git repository.
kadir pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push:
new 54f2c93 PHOENIX-5807 Index rows without empty column should be treated as unverified
54f2c93 is described below
commit 54f2c93176ce57a077769a6f9740345c1da3ac45
Author: Kadir <ko...@salesforce.com>
AuthorDate: Sun Mar 29 17:41:37 2020 -0700
PHOENIX-5807 Index rows without empty column should be treated as unverified
---
.../end2end/index/GlobalIndexCheckerIT.java | 43 ++++++++++++++++++++++
.../coprocessor/IndexRebuildRegionScanner.java | 2 +-
.../apache/phoenix/index/GlobalIndexChecker.java | 23 ++----------
3 files changed, 47 insertions(+), 21 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 8fbeae0..ae3da0b 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
@@ -40,9 +40,11 @@ import org.apache.phoenix.hbase.index.IndexRegionObserver;
import org.apache.phoenix.mapreduce.index.IndexTool;
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.schema.PTableImpl;
+import org.apache.phoenix.util.EnvironmentEdgeManager;
import org.apache.phoenix.util.PhoenixRuntime;
import org.apache.phoenix.util.QueryUtil;
import org.apache.phoenix.util.ReadOnlyProps;
+import org.apache.phoenix.util.TestUtil;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -164,6 +166,47 @@ public class GlobalIndexCheckerIT extends BaseUniqueNamesOwnClusterIT {
}
@Test
+ public void testIndexRowWithoutEmptyColumn() throws Exception {
+ if (async) {
+ // No need to run the same test twice one for async = true and the other for async = false
+ return;
+ }
+ long scn;
+ String dataTableName = generateUniqueName();
+ String indexTableName = generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ populateTable(dataTableName); // with two rows ('a', 'ab', 'abc', 'abcd') and ('b', 'bc', 'bcd', 'bcde')
+ conn.createStatement().execute("CREATE INDEX " + indexTableName + " on " +
+ dataTableName + " (val1) include (val2, val3)");
+ scn = EnvironmentEdgeManager.currentTimeMillis();
+ // Configure IndexRegionObserver to fail the data write phase
+ IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+ conn.createStatement().execute("upsert into " + dataTableName + " values ('a', 'abc','abcc', 'abccd')");
+ commitWithException(conn);
+ // The above upsert will create an unverified index row with row key {'abc', 'a'} and will make the existing
+ // index row with row key {'ab', 'a'} unverified
+ // Configure IndexRegionObserver to allow the data write phase
+ IndexRegionObserver.setFailDataTableUpdatesForTesting(false);
+ // Do major compaction which will remove the empty column cell from the index row with row key {'ab', 'a'}
+ TestUtil.doMajorCompaction(conn, indexTableName);
+ }
+ Properties props = new Properties();
+ props.setProperty("CurrentSCN", Long.toString(scn));
+ try (Connection connWithSCN = DriverManager.getConnection(getUrl(), props)) {
+ String selectSql = "SELECT * from " + dataTableName + " WHERE val1 = 'ab'";
+ // Verify that we will read from the index table
+ assertExplainPlan(connWithSCN, selectSql, dataTableName, indexTableName);
+ ResultSet rs = connWithSCN.createStatement().executeQuery(selectSql);
+ assertTrue(rs.next());
+ assertEquals("a", rs.getString(1));
+ assertEquals("ab", rs.getString(2));
+ assertEquals("abc", rs.getString(3));
+ assertEquals("abcd", rs.getString(4));
+ assertFalse(rs.next());
+ }
+ }
+
+ @Test
public void testSimulateConcurrentUpdates() throws Exception {
try (Connection conn = DriverManager.getConnection(getUrl())) {
String dataTableName = generateUniqueName();
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
index 5f3b719..ee2cf4f 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
@@ -570,7 +570,7 @@ public class IndexRebuildRegionScanner extends BaseRegionScanner {
indexMaintainer.getEmptyKeyValueQualifier());
Cell cell = (cellList != null && !cellList.isEmpty()) ? cellList.get(0) : null;
if (cell == null) {
- throw new DoNotRetryIOException("No empty column cell");
+ return false;
}
if (Bytes.compareTo(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength(),
VERIFIED_BYTES, 0, VERIFIED_BYTES.length) == 0) {
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 b133843..b0d0db5 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
@@ -38,7 +38,6 @@ import org.apache.hadoop.hbase.CoprocessorEnvironment;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Delete;
-import org.apache.hadoop.hbase.client.Get;
import org.apache.hadoop.hbase.client.Mutation;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.Result;
@@ -411,22 +410,6 @@ public class GlobalIndexChecker implements RegionCoprocessor, RegionObserver {
emptyCQ, 0, emptyCQ.length) == 0;
}
- private boolean verifyRow(byte[] rowKey) throws IOException {
- LOG.warn("Scan " + scan + " did not return the empty column for " + region.getRegionInfo().getTable().getNameAsString());
- Get get = new Get(rowKey);
- get.setTimeRange(minTimestamp, maxTimestamp);
- get.addColumn(emptyCF, emptyCQ);
- Result result = region.get(get);
- if (result.isEmpty()) {
- throw new DoNotRetryIOException("The empty column does not exist in a row in " + region.getRegionInfo().getTable().getNameAsString());
- }
- if (Bytes.compareTo(result.getValue(emptyCF, emptyCQ), 0, VERIFIED_BYTES.length,
- VERIFIED_BYTES, 0, VERIFIED_BYTES.length) != 0) {
- return false;
- }
- return true;
- }
-
/**
* An index row is composed of cells with the same timestamp. However, if there are multiple versions of an
* index row, HBase can return an index row with cells from multiple versions, and thus it can return cells
@@ -465,7 +448,6 @@ public class GlobalIndexChecker implements RegionCoprocessor, RegionObserver {
}
}
-
private boolean verifyRowAndRemoveEmptyColumn(List<Cell> cellList) throws IOException {
if (!indexMaintainer.isImmutableRows()) {
removeOlderCells(cellList);
@@ -491,8 +473,9 @@ public class GlobalIndexChecker implements RegionCoprocessor, RegionObserver {
return true;
}
}
- byte[] rowKey = CellUtil.cloneRow(cell);
- return verifyRow(rowKey);
+ // This index row does not have an empty column cell. It must be removed by compaction. This row will
+ // be treated as unverified so that it can be repaired
+ return false;
}
private long getMaxTimestamp(List<Cell> cellList) {