tkhurana commented on a change in pull request #995:
URL: https://github.com/apache/phoenix/pull/995#discussion_r536337736



##########
File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexRepairRegionScannerIT.java
##########
@@ -163,35 +308,251 @@ public void testRepairExtraIndexRows() throws Exception {
         String indexTableName = generateUniqueName();
         String indexTableFullName = SchemaUtil.getTableName(schemaName, 
indexTableName);
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            initTablesAndAddExtraRowsToIndex(conn, schemaName, dataTableName, 
indexTableName, NROWS);
+
+            // do index rebuild without -fi and check with scrutiny that index 
tool failed to fix the extra rows
+            IndexToolIT.runIndexTool(false, false, schemaName, dataTableName,
+                indexTableName, null, 0, IndexVerifyType.BEFORE);
+
+            boolean failed;
+            try {
+                IndexScrutiny.scrutinizeIndex(conn, dataTableFullName, 
indexTableFullName);
+                failed = false;
+            } catch (AssertionError e) {
+                failed = true;
+            }
+            assertTrue(failed);
+
+            // now repair the index with -fi
+            IndexTool indexTool = IndexToolIT.runIndexTool(false, false, 
schemaName, dataTableName,
+                indexTableName, null, 0, IndexVerifyType.BEFORE, "-fi");
+
+            long actualRowCount = IndexScrutiny.scrutinizeIndex(conn, 
dataTableFullName, indexTableFullName);
+            assertEquals(NROWS, actualRowCount);
+
+            assertExtraCounters(indexTool, NROWS, 0, true);
+        }
+    }
+
+    @Test
+    public void testRepairExtraIndexRows_PostIndexUpdateFailure_overwrite() 
throws Exception {
+        if (!mutable) {
+            return;
+        }
+        final int NROWS = 4;
+        String schemaName = generateUniqueName();
+        String dataTableName = generateUniqueName();
+        String dataTableFullName = SchemaUtil.getTableName(schemaName, 
dataTableName);
+        String indexTableName = generateUniqueName();
+        String indexTableFullName = SchemaUtil.getTableName(schemaName, 
indexTableName);
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
         try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
             conn.createStatement().execute("CREATE TABLE " + dataTableFullName
-                    + " (ID INTEGER NOT NULL PRIMARY KEY, VAL1 INTEGER, VAL2 
INTEGER) "
-                    + tableDDLOptions);
+                + " (ID INTEGER NOT NULL PRIMARY KEY, VAL1 INTEGER, VAL2 
INTEGER) " + tableDDLOptions);
+            conn.createStatement().execute(String.format(
+                "CREATE INDEX %s ON %s (VAL1) INCLUDE (VAL2)", indexTableName, 
dataTableFullName));
+
             PreparedStatement dataPreparedStatement =
-                    conn.prepareStatement("UPSERT INTO " + dataTableFullName + 
" VALUES(?,?,?)");
+                conn.prepareStatement("UPSERT INTO " + dataTableFullName + " 
VALUES(?,?,?)");
             for (int i = 1; i <= NROWS; i++) {
                 dataPreparedStatement.setInt(1, i);
                 dataPreparedStatement.setInt(2, i + 1);
                 dataPreparedStatement.setInt(3, i * 2);
                 dataPreparedStatement.execute();
             }
             conn.commit();
+
+            IndexRegionObserver.setFailPostIndexUpdatesForTesting(true);
+            conn.createStatement().execute("UPSERT INTO " + dataTableFullName 
+ " VALUES(3, 100, 200)");
+            conn.commit();
+            IndexRegionObserver.setFailPostIndexUpdatesForTesting(false);
+
+            IndexTool indexTool = IndexToolIT.runIndexTool(false, false, 
schemaName, dataTableName,
+                indexTableName, null, 0, IndexVerifyType.BEFORE, "-fi");
+
+            CounterGroup mrJobCounters = 
IndexToolIT.getMRJobCounters(indexTool);
+            assertEquals(2,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_INVALID_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(2,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_VERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+
+            indexTool = IndexToolIT.runIndexTool(false, false, schemaName, 
dataTableName,
+                indexTableName, null, 0, IndexVerifyType.ONLY, "-fi");
+            mrJobCounters = IndexToolIT.getMRJobCounters(indexTool);
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_INVALID_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_VERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+
+            long actualRowCount = IndexScrutiny.scrutinizeIndex(conn, 
dataTableFullName, indexTableFullName);
+            assertEquals(NROWS, actualRowCount);
+        }
+    }
+
+    @Test
+    public void testRepairExtraIndexRows_PostIndexUpdateFailure_delete() 
throws Exception {
+        if (!mutable) {
+            return;
+        }
+        final int NROWS = 4;
+        String schemaName = generateUniqueName();
+        String dataTableName = generateUniqueName();
+        String dataTableFullName = SchemaUtil.getTableName(schemaName, 
dataTableName);
+        String indexTableName = generateUniqueName();
+        String indexTableFullName = SchemaUtil.getTableName(schemaName, 
indexTableName);
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            conn.createStatement().execute("CREATE TABLE " + dataTableFullName
+                + " (ID INTEGER NOT NULL PRIMARY KEY, VAL1 INTEGER, VAL2 
INTEGER) " + tableDDLOptions);
             conn.createStatement().execute(String.format(
-                    "CREATE INDEX %s ON %s (VAL1) INCLUDE (VAL2)", 
indexTableName, dataTableFullName));
-            // Add extra index rows
-            PreparedStatement indexPreparedStatement =
-                    conn.prepareStatement("UPSERT INTO " + indexTableFullName 
+ " VALUES(?,?,?)");
+                "CREATE INDEX %s ON %s (VAL1) INCLUDE (VAL2)", indexTableName, 
dataTableFullName));
 
-            for (int i = NROWS + 1; i <= 2 * NROWS; i++) {
-                indexPreparedStatement.setInt(1, i + 1); // the indexed column
-                indexPreparedStatement.setInt(2, i); // the data pk column
-                indexPreparedStatement.setInt(3, i * 2); // the included column
-                indexPreparedStatement.execute();
+            PreparedStatement dataPreparedStatement =
+                conn.prepareStatement("UPSERT INTO " + dataTableFullName + " 
VALUES(?,?,?)");
+            for (int i = 1; i <= NROWS; i++) {
+                dataPreparedStatement.setInt(1, i);
+                dataPreparedStatement.setInt(2, i + 1);
+                dataPreparedStatement.setInt(3, i * 2);
+                dataPreparedStatement.execute();
             }
             conn.commit();
-            // Set all index row statuses to verified so that read verify will 
not fix them. We want them to be fixed
-            // by IndexRepairRegionScanner
-            setIndexRowStatusesToVerified(conn, dataTableFullName, 
indexTableFullName);
+
+            IndexRegionObserver.setFailPostIndexUpdatesForTesting(true);
+            conn.createStatement().execute("DELETE FROM " + dataTableFullName 
+ " WHERE ID = 3");
+            conn.commit();
+            IndexRegionObserver.setFailPostIndexUpdatesForTesting(false);
+            TestUtil.doMajorCompaction(conn, dataTableFullName);
+
+            IndexTool indexTool = IndexToolIT.runIndexTool(false, false, 
schemaName, dataTableName,
+                indexTableName, null, 0, IndexVerifyType.BEFORE, "-fi");
+
+            CounterGroup mrJobCounters = 
IndexToolIT.getMRJobCounters(indexTool);
+
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_INVALID_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_VERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(1,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+
+            indexTool = IndexToolIT.runIndexTool(false, false, schemaName, 
dataTableName,
+                indexTableName, null, 0, IndexVerifyType.ONLY, "-fi");
+            mrJobCounters = IndexToolIT.getMRJobCounters(indexTool);
+
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_INVALID_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REBUILD_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_VERIFIED_INDEX_ROW_COUNT.name()).getValue());
+            assertEquals(0,
+                
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+
+            long actualRowCount = IndexScrutiny.scrutinizeIndex(conn, 
dataTableFullName, indexTableFullName);
+            assertEquals(NROWS - 1, actualRowCount);
+        }
+    }
+
+    @Test
+    public void testRepairExtraIndexRows_DataTableUpdateFailure() throws 
Exception {
+        if (!mutable) {
+            return;
+        }
+        final int NROWS = 20;
+        String schemaName = generateUniqueName();
+        String dataTableName = generateUniqueName();
+        String dataTableFullName = SchemaUtil.getTableName(schemaName, 
dataTableName);
+        String indexTableName = generateUniqueName();
+        String indexTableFullName = SchemaUtil.getTableName(schemaName, 
indexTableName);
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            conn.createStatement().execute("CREATE TABLE " + dataTableFullName
+                + " (ID INTEGER NOT NULL PRIMARY KEY, VAL1 INTEGER, VAL2 
INTEGER) " + tableDDLOptions);
+            conn.createStatement().execute(String.format(
+                "CREATE INDEX %s ON %s (VAL1) INCLUDE (VAL2)", indexTableName, 
dataTableFullName));
+
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+
+            PreparedStatement dataPreparedStatement =
+                conn.prepareStatement("UPSERT INTO " + dataTableFullName + " 
VALUES(?,?,?)");
+            for (int i = 1; i <= NROWS; i++) {
+                dataPreparedStatement.setInt(1, i);
+                dataPreparedStatement.setInt(2, i + 1);
+                dataPreparedStatement.setInt(3, i * 2);
+                dataPreparedStatement.execute();
+            }
+            commitWithException(conn);
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+
+            IndexTool indexTool = IndexToolIT.runIndexTool(false, false, 
schemaName, dataTableName,
+                indexTableName, null, 0, IndexVerifyType.BEFORE, "-fi");
+
+            long actualRowCount = IndexScrutiny.scrutinizeIndex(conn, 
dataTableFullName, indexTableFullName);
+            assertEquals(0, actualRowCount);
+
+            assertExtraCounters(indexTool, 0, NROWS, true);
+        }
+    }
+
+    @Test
+    public void testPITRow() throws Exception {
+        final int NROWS = 1;
+        String schemaName = generateUniqueName();
+        String dataTableName = generateUniqueName();
+        String dataTableFullName = SchemaUtil.getTableName(schemaName, 
dataTableName);
+        String indexTableName = generateUniqueName();
+        String indexTableFullName = SchemaUtil.getTableName(schemaName, 
indexTableName);
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            initTablesAndAddExtraRowsToIndex(conn, schemaName, dataTableName, 
indexTableName, NROWS);
+
+            IndexToolIT.runIndexTool(false, false, schemaName, dataTableName,
+                indexTableName, null, 0, IndexVerifyType.ONLY, "-fi");
+
+            Cell cell = 
IndexToolIT.getErrorMessageFromIndexToolOutputTable(conn, dataTableFullName, 
indexTableFullName);
+            try {
+                String expectedErrorMsg = 
IndexRepairRegionScanner.ERROR_MESSAGE_EXTRA_INDEX_ROW;
+                String actualErrorMsg = Bytes.toString(cell.getValueArray(), 
cell.getValueOffset(), cell.getValueLength());
+                assertTrue(expectedErrorMsg.equals(actualErrorMsg));
+            } catch(Exception ex) {
+                Assert.fail("Fail to parsing the error message from 
IndexToolOutputTable");
+            }
+        }
+    }
+
+    @Test
+    public void testVerifyAfterExtraIndexRows() throws Exception {
+        final int NROWS = 20;
+        String schemaName = generateUniqueName();
+        String dataTableName = generateUniqueName();
+        String dataTableFullName = SchemaUtil.getTableName(schemaName, 
dataTableName);
+        String indexTableName = generateUniqueName();
+        String indexTableFullName = SchemaUtil.getTableName(schemaName, 
indexTableName);
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            initTablesAndAddExtraRowsToIndex(conn, schemaName, dataTableName, 
indexTableName, NROWS);
+
+            // Run -v AFTER and check it doesn't fix the extra rows and the 
job fails
+            IndexTool indexTool = IndexToolIT.runIndexTool(false, false, 
schemaName, dataTableName,
+                indexTableName, null, -1, IndexVerifyType.AFTER, "-fi");

Review comment:
       @gokcen AFTER first rebuilds the index using the expected mutations from 
the data table. So any extra rows in the index table will remain after the 
rebuild since they are not present in the data table. Then when we verify, 
those rows will be reported as extra. As a result, the AFTER job will fail.




----------------------------------------------------------------
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]


Reply via email to