PHOENIX-4975 Fix failing unit tests for Omid due to shadow cells and no local indexes
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/50c2a3be Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/50c2a3be Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/50c2a3be Branch: refs/heads/4.x-cdh5.15 Commit: 50c2a3beee324823bd1a21459b62fbee1e1eca40 Parents: a4453b6 Author: James Taylor <jamestay...@apache.org> Authored: Tue Oct 16 17:16:43 2018 +0100 Committer: Pedro Boado <pbo...@apache.org> Committed: Wed Oct 17 22:50:43 2018 +0100 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/IndexToolIT.java | 16 ++++-- .../StatsEnabledSplitSystemCatalogIT.java | 34 +++++++----- .../java/org/apache/phoenix/end2end/ViewIT.java | 18 +++++-- .../phoenix/schema/stats/StatsCollectorIT.java | 54 ++++++++++++-------- 4 files changed, 82 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/50c2a3be/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java index b8372c4..c99f145 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java @@ -49,6 +49,8 @@ import org.apache.phoenix.jdbc.PhoenixConnection; import org.apache.phoenix.mapreduce.index.IndexTool; import org.apache.phoenix.query.QueryServices; import org.apache.phoenix.query.QueryServicesOptions; +import org.apache.phoenix.transaction.PhoenixTransactionProvider.Feature; +import org.apache.phoenix.transaction.TransactionFactory; import org.apache.phoenix.util.PropertiesUtil; import org.apache.phoenix.util.QueryUtil; import org.apache.phoenix.util.ReadOnlyProps; @@ -111,12 +113,18 @@ public class IndexToolIT extends ParallelStatsEnabledIT { public static Collection<Object[]> data() { List<Object[]> list = Lists.newArrayListWithExpectedSize(48); boolean[] Booleans = new boolean[] { false, true }; - for (Object transactionProvider : new String[] {"TEPHRA", "OMID", null}) { + for (String transactionProvider : new String[] {"TEPHRA", "OMID", null}) { for (boolean mutable : Booleans) { for (boolean localIndex : Booleans) { - for (boolean directApi : Booleans) { - for (boolean useSnapshot : Booleans) { - list.add(new Object[] { transactionProvider, mutable, localIndex, directApi, useSnapshot }); + if (!localIndex + || transactionProvider == null + || !TransactionFactory.getTransactionProvider( + TransactionFactory.Provider.valueOf(transactionProvider)) + .isUnsupported(Feature.ALLOW_LOCAL_INDEX)) { + for (boolean directApi : Booleans) { + for (boolean useSnapshot : Booleans) { + list.add(new Object[] { transactionProvider, mutable, localIndex, directApi, useSnapshot }); + } } } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/50c2a3be/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsEnabledSplitSystemCatalogIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsEnabledSplitSystemCatalogIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsEnabledSplitSystemCatalogIT.java index 197263f..0a0dd21 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsEnabledSplitSystemCatalogIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsEnabledSplitSystemCatalogIT.java @@ -34,10 +34,10 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.client.HTableInterface; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; import org.apache.phoenix.exception.SQLExceptionCode; @@ -45,6 +45,8 @@ import org.apache.phoenix.jdbc.PhoenixConnection; import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.query.QueryServices; import org.apache.phoenix.schema.ReadOnlyTableException; +import org.apache.phoenix.transaction.PhoenixTransactionProvider.Feature; +import org.apache.phoenix.transaction.TransactionFactory; import org.apache.phoenix.util.ReadOnlyProps; import org.apache.phoenix.util.ScanUtil; import org.apache.phoenix.util.SchemaUtil; @@ -61,20 +63,20 @@ import com.google.common.collect.Maps; public class StatsEnabledSplitSystemCatalogIT extends BaseUniqueNamesOwnClusterIT { private String tableDDLOptions; - private boolean transactional; + private String transactionProvider; - public StatsEnabledSplitSystemCatalogIT(boolean transactional) { + public StatsEnabledSplitSystemCatalogIT(String transactionProvider) { StringBuilder optionBuilder = new StringBuilder(); - this.transactional = transactional; - if (transactional) { - optionBuilder.append(" TRANSACTIONAL=true "); + this.transactionProvider = transactionProvider; + if (transactionProvider != null) { + optionBuilder.append(" TRANSACTIONAL=true, TRANSACTION_PROVIDER='" + transactionProvider + "'"); } this.tableDDLOptions = optionBuilder.toString(); } @Parameters(name = "transactional = {0}") - public static Collection<Boolean> data() { - return Arrays.asList(new Boolean[] { false, true }); + public static Collection<Object> data() { + return Arrays.asList(new Object[] { null, "TEPHRA", "OMID" }); } @BeforeClass @@ -101,7 +103,11 @@ public class StatsEnabledSplitSystemCatalogIT extends BaseUniqueNamesOwnClusterI @Test public void testSaltedUpdatableViewWithLocalIndex() throws Exception { - testUpdatableViewWithIndex(3, true); + if (transactionProvider == null || + !TransactionFactory.getTransactionProvider( + TransactionFactory.Provider.valueOf(transactionProvider)).isUnsupported(Feature.ALLOW_LOCAL_INDEX)) { + testUpdatableViewWithIndex(3, true); + } } @Test @@ -111,7 +117,11 @@ public class StatsEnabledSplitSystemCatalogIT extends BaseUniqueNamesOwnClusterI @Test public void testNonSaltedUpdatableViewWithLocalIndex() throws Exception { - testUpdatableViewWithIndex(null, true); + if (transactionProvider == null || + !TransactionFactory.getTransactionProvider( + TransactionFactory.Provider.valueOf(transactionProvider)).isUnsupported(Feature.ALLOW_LOCAL_INDEX)) { + testUpdatableViewWithIndex(null, true); + } } @Test @@ -165,7 +175,7 @@ public class StatsEnabledSplitSystemCatalogIT extends BaseUniqueNamesOwnClusterI // Confirm that dropping the view also deletes the rows in the index if (saltBuckets == null) { try (Connection conn = DriverManager.getConnection(getUrl())) { - HTableInterface htable = conn.unwrap(PhoenixConnection.class).getQueryServices() + Table htable = conn.unwrap(PhoenixConnection.class).getQueryServices() .getTable(Bytes.toBytes(physicalTableName)); if (ScanUtil.isLocalIndex(scan)) { ScanUtil.setLocalIndexAttributes(scan, 0, HConstants.EMPTY_BYTE_ARRAY, HConstants.EMPTY_BYTE_ARRAY, @@ -212,7 +222,7 @@ public class StatsEnabledSplitSystemCatalogIT extends BaseUniqueNamesOwnClusterI } conn.commit(); - analyzeTable(conn, fullParentViewName, transactional); + analyzeTable(conn, fullParentViewName, transactionProvider != null); List<KeyRange> splits = getAllSplits(conn, fullParentViewName); assertEquals(4, splits.size()); http://git-wip-us.apache.org/repos/asf/phoenix/blob/50c2a3be/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java index cad897f..090ccaa 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java @@ -80,6 +80,8 @@ import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.PTableType; import org.apache.phoenix.schema.ReadOnlyTableException; import org.apache.phoenix.schema.TableNotFoundException; +import org.apache.phoenix.transaction.PhoenixTransactionProvider.Feature; +import org.apache.phoenix.transaction.TransactionFactory; import org.apache.phoenix.util.MetaDataUtil; import org.apache.phoenix.util.PhoenixRuntime; import org.apache.phoenix.util.PropertiesUtil; @@ -464,9 +466,13 @@ public class ViewIT extends SplitSystemCatalogIT { conn.createStatement().execute(tableDdl); String ddl = "CREATE VIEW " + fullViewName1 + " (v2 VARCHAR) AS SELECT * FROM " + fullTableName + " WHERE k > 5"; conn.createStatement().execute(ddl); - String indexName = generateUniqueName(); - ddl = "CREATE LOCAL INDEX " + indexName + " on " + fullViewName1 + "(v2)"; - conn.createStatement().execute(ddl); + if (transactionProvider == null || + !TransactionFactory.getTransactionProvider( + TransactionFactory.Provider.valueOf(transactionProvider)).isUnsupported(Feature.ALLOW_LOCAL_INDEX)) { + String indexName = generateUniqueName(); + ddl = "CREATE LOCAL INDEX " + indexName + " on " + fullViewName1 + "(v2)"; + conn.createStatement().execute(ddl); + } ddl = "CREATE VIEW " + fullViewName2 + "(v2 VARCHAR) AS SELECT * FROM " + fullTableName + " WHERE k > 10"; conn.createStatement().execute(ddl); @@ -655,7 +661,11 @@ public class ViewIT extends SplitSystemCatalogIT { @Test public void testViewUsesTableLocalIndex() throws Exception { - testViewUsesTableIndex(true); + if (transactionProvider == null || + !TransactionFactory.getTransactionProvider( + TransactionFactory.Provider.valueOf(transactionProvider)).isUnsupported(Feature.ALLOW_LOCAL_INDEX)) { + testViewUsesTableIndex(true); + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/50c2a3be/phoenix-core/src/it/java/org/apache/phoenix/schema/stats/StatsCollectorIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/schema/stats/StatsCollectorIT.java b/phoenix-core/src/it/java/org/apache/phoenix/schema/stats/StatsCollectorIT.java index 8fd6d75..0caf61a 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/schema/stats/StatsCollectorIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/schema/stats/StatsCollectorIT.java @@ -60,6 +60,8 @@ import org.apache.phoenix.query.QueryServicesOptions; import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.PTableImpl; import org.apache.phoenix.schema.PTableKey; +import org.apache.phoenix.transaction.PhoenixTransactionProvider.Feature; +import org.apache.phoenix.transaction.TransactionFactory; import org.apache.phoenix.util.MetaDataUtil; import org.apache.phoenix.util.PropertiesUtil; import org.apache.phoenix.util.QueryUtil; @@ -85,9 +87,11 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { private String physicalTableName; private final boolean userTableNamespaceMapped; private final boolean mutable; + private final String transactionProvider; private static final int defaultGuidePostWidth = 20; protected StatsCollectorIT(boolean mutable, String transactionProvider, boolean userTableNamespaceMapped, boolean columnEncoded) { + this.transactionProvider = transactionProvider; StringBuilder sb = new StringBuilder(); if (columnEncoded) { sb.append("COLUMN_ENCODED_BYTES=4"); @@ -179,7 +183,7 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { rs = conn.createStatement().executeQuery("EXPLAIN SELECT v2 FROM " + fullTableName + " WHERE v2='foo'"); explainPlan = QueryUtil.getExplainPlan(rs); // if we are using the ONE_CELL_PER_COLUMN_FAMILY storage scheme, we will have the single kv even though there are no values for col family v2 - String stats = columnEncoded && !mutable ? "4-CHUNK 1 ROWS 38 BYTES" : "3-CHUNK 0 ROWS 20 BYTES"; + String stats = columnEncoded && !mutable ? "4-CHUNK 1 ROWS 38 BYTES" : "3-CHUNK 0 ROWS 20 BYTES"; assertEquals( "CLIENT " + stats + " PARALLEL 3-WAY FULL SCAN OVER " + physicalTableName + "\n" + " SERVER FILTER BY B.V2 = 'foo'\n" + @@ -188,7 +192,7 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + fullTableName); explainPlan = QueryUtil.getExplainPlan(rs); assertEquals( - "CLIENT 4-CHUNK 1 ROWS " + (columnEncoded ? "28" : "34") + " BYTES PARALLEL 3-WAY FULL SCAN OVER " + physicalTableName + "\n" + + "CLIENT 4-CHUNK 1 ROWS " + (columnEncoded ? "28" : TransactionFactory.Provider.OMID.name().equals(transactionProvider) ? "38" : "34") + " BYTES PARALLEL 3-WAY FULL SCAN OVER " + physicalTableName + "\n" + "CLIENT MERGE SORT", explainPlan); rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + fullTableName + " WHERE k = 'a'"); @@ -527,7 +531,7 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { List<KeyRange> keyRanges = getAllSplits(conn, fullTableName); assertEquals(26, keyRanges.size()); rs = conn.createStatement().executeQuery("EXPLAIN SELECT * FROM " + fullTableName); - assertEquals("CLIENT 26-CHUNK 25 ROWS " + (columnEncoded ? ( mutable ? "12530" : "13902" ) : "12420") + " BYTES PARALLEL 1-WAY FULL SCAN OVER " + physicalTableName, + assertEquals("CLIENT 26-CHUNK 25 ROWS " + (columnEncoded ? ( mutable ? "12530" : "13902" ) : (TransactionFactory.Provider.OMID.name().equals(transactionProvider)) ? "25044" : "12420") + " BYTES PARALLEL 1-WAY FULL SCAN OVER " + physicalTableName, QueryUtil.getExplainPlan(rs)); ConnectionQueryServices services = conn.unwrap(PhoenixConnection.class).getQueryServices(); @@ -540,7 +544,8 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { conn.createStatement().execute(query); keyRanges = getAllSplits(conn, fullTableName); boolean oneCellPerColFamliyStorageScheme = !mutable && columnEncoded; - assertEquals(oneCellPerColFamliyStorageScheme ? 13 : 12, keyRanges.size()); + boolean hasShadowCells = TransactionFactory.Provider.OMID.name().equals(transactionProvider); + assertEquals(oneCellPerColFamliyStorageScheme ? 13 : hasShadowCells ? 23 : 12, keyRanges.size()); rs = conn .createStatement() @@ -551,26 +556,26 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { assertTrue(rs.next()); assertEquals("A", rs.getString(1)); assertEquals(24, rs.getInt(2)); - assertEquals(columnEncoded ? ( mutable ? 12252 : 13624 ) : 12144, rs.getInt(3)); - assertEquals(oneCellPerColFamliyStorageScheme ? 12 : 11, rs.getInt(4)); + assertEquals(columnEncoded ? ( mutable ? 12252 : 13624 ) : hasShadowCells ? 24756 : 12144, rs.getInt(3)); + assertEquals(oneCellPerColFamliyStorageScheme ? 12 : hasShadowCells ? 22 : 11, rs.getInt(4)); assertTrue(rs.next()); assertEquals("B", rs.getString(1)); assertEquals(oneCellPerColFamliyStorageScheme ? 24 : 20, rs.getInt(2)); - assertEquals(columnEncoded ? ( mutable ? 5600 : 6972 ) : 5540, rs.getInt(3)); - assertEquals(oneCellPerColFamliyStorageScheme ? 6 : 5, rs.getInt(4)); + assertEquals(columnEncoded ? ( mutable ? 5600 : 6972 ) : hasShadowCells ? 11260 : 5540, rs.getInt(3)); + assertEquals(oneCellPerColFamliyStorageScheme ? 6 : hasShadowCells ? 10 : 5, rs.getInt(4)); assertTrue(rs.next()); assertEquals("C", rs.getString(1)); assertEquals(24, rs.getInt(2)); - assertEquals(columnEncoded ? ( mutable ? 6724 : 6988 ) : 6652, rs.getInt(3)); - assertEquals(6, rs.getInt(4)); + assertEquals(columnEncoded ? ( mutable ? 6724 : 6988 ) : hasShadowCells ? 13520 : 6652, rs.getInt(3)); + assertEquals(hasShadowCells ? 12 : 6, rs.getInt(4)); assertTrue(rs.next()); assertEquals("D", rs.getString(1)); assertEquals(24, rs.getInt(2)); - assertEquals(columnEncoded ? ( mutable ? 6724 : 6988 ) : 6652, rs.getInt(3)); - assertEquals(6, rs.getInt(4)); + assertEquals(columnEncoded ? ( mutable ? 6724 : 6988 ) : hasShadowCells ? 13520 : 6652, rs.getInt(3)); + assertEquals(hasShadowCells ? 12 : 6, rs.getInt(4)); assertFalse(rs.next()); @@ -610,6 +615,7 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { conn.createStatement().execute(query); Random r = new Random(); int count = 0; + boolean hasShadowCells = TransactionFactory.Provider.OMID.name().equals(transactionProvider); while (count < 4) { int startIndex = r.nextInt(strings.length); int endIndex = r.nextInt(strings.length - startIndex) + startIndex; @@ -625,7 +631,11 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { assertTrue(rs.next()); assertEquals("C2", rs.getString(1)); assertEquals(rows, rs.getLong(2)); - assertEquals(c2Bytes, rs.getLong(3)); + // OMID with the shadow cells it creates will have more bytes, but getting + // an exact byte count based on the number or rows is not possible because + // it is variable on a row-by-row basis. + long sumOfGuidePostsWidth = rs.getLong(3); + assertTrue(hasShadowCells ? sumOfGuidePostsWidth > 2 * c2Bytes && sumOfGuidePostsWidth <= 3 * c2Bytes: rs.getLong(3) == c2Bytes); count++; } } @@ -793,13 +803,17 @@ public abstract class StatsCollectorIT extends BaseUniqueNamesOwnClusterIT { assertEquals(defaultGuidePostWidth, statsCollector.getGuidePostDepth()); // let's check out local index too - String localIndex = "LI_" + generateUniqueName(); - ddl = "CREATE LOCAL INDEX " + localIndex + " ON " + baseTable + " (b) INCLUDE (c) "; - conn.createStatement().execute(ddl); - // local indexes reside on the same table as base data table - statsCollector = getDefaultStatsCollectorForTable(baseTable); - statsCollector.init(); - assertEquals(defaultGuidePostWidth, statsCollector.getGuidePostDepth()); + if (transactionProvider == null || + !TransactionFactory.getTransactionProvider( + TransactionFactory.Provider.valueOf(transactionProvider)).isUnsupported(Feature.ALLOW_LOCAL_INDEX)) { + String localIndex = "LI_" + generateUniqueName(); + ddl = "CREATE LOCAL INDEX " + localIndex + " ON " + baseTable + " (b) INCLUDE (c) "; + conn.createStatement().execute(ddl); + // local indexes reside on the same table as base data table + statsCollector = getDefaultStatsCollectorForTable(baseTable); + statsCollector.init(); + assertEquals(defaultGuidePostWidth, statsCollector.getGuidePostDepth()); + } // now let's create a view and an index on it and see what guide post width is used for // it
