You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by tk...@apache.org on 2023/07/28 19:39:49 UTC

[phoenix] branch 5.1 updated: PHOENIX-6897 Filters on unverified index rows return wrong result (#1… (#1632)

This is an automated email from the ASF dual-hosted git repository.

tkhurana pushed a commit to branch 5.1
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/5.1 by this push:
     new 13826bcca3 PHOENIX-6897 Filters on unverified index rows return wrong result (#1… (#1632)
13826bcca3 is described below

commit 13826bcca380cad95193d68f1de2622c4315bf57
Author: tkhurana <kh...@gmail.com>
AuthorDate: Fri Jul 28 12:39:39 2023 -0700

    PHOENIX-6897 Filters on unverified index rows return wrong result (#1… (#1632)
    
    * PHOENIX-6897 Filters on unverified index rows return wrong result (#1597)
    
    * PHOENIX-6897 Filters on unverified index rows return wrong result
    
    * Fixed checkstyle and missing license warnings
    
    * Addressed review comments
---
 .../end2end/index/GlobalIndexCheckerIT.java        | 200 +++++++++++++++++++++
 .../apache/phoenix/filter/UnverifiedRowFilter.java | 130 ++++++++++++++
 .../apache/phoenix/index/GlobalIndexChecker.java   |  80 ++++++---
 3 files changed, 389 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 2b8513dbb9..d44e7e5fb7 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
@@ -29,6 +29,7 @@ import static org.apache.phoenix.mapreduce.index.PhoenixIndexToolJobCounters.BEF
 import static org.apache.phoenix.mapreduce.index.PhoenixIndexToolJobCounters.BEFORE_REBUILD_VALID_INDEX_ROW_COUNT;
 import static org.apache.phoenix.mapreduce.index.PhoenixIndexToolJobCounters.REBUILT_INDEX_ROW_COUNT;
 import static org.apache.phoenix.mapreduce.index.PhoenixIndexToolJobCounters.SCANNED_DATA_ROW_COUNT;
+import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -45,6 +46,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
+import org.apache.hadoop.hbase.TableName;
 import org.apache.phoenix.thirdparty.com.google.common.base.Strings;
 import org.apache.phoenix.thirdparty.com.google.common.collect.Maps;
 import org.apache.phoenix.end2end.IndexToolIT;
@@ -56,6 +58,7 @@ 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.PropertiesUtil;
 import org.apache.phoenix.util.QueryUtil;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.apache.phoenix.util.TestUtil;
@@ -1021,6 +1024,203 @@ public class GlobalIndexCheckerIT extends BaseTest {
         }
     }
 
+    @Test
+    public void testUnverifiedIndexRowWithFilter() throws Exception {
+        if (async) {
+            // No need to run the same test twice one for async = true and the other for async = false
+            return;
+        }
+        // running this test with PagingFilter disabled
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        props.setProperty(QueryServices.PHOENIX_SERVER_PAGING_ENABLED_ATTRIB,
+                String.valueOf(false));
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String dataTableName = generateUniqueName();
+            String indexName = generateUniqueName();
+            conn.createStatement().execute("create table " + dataTableName +
+                    " (id integer primary key, name varchar, status integer, val varchar)" + tableDDLOptions);
+            conn.commit();
+            conn.createStatement().execute("create index " + indexName +
+                    " ON " + dataTableName + "(name) include (status, val)");
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " values (1, 'tom', 1, 'blah')");
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " values (2, 'jerry', 2, 'jee')");
+            conn.commit();
+
+            // fail phase 2
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+            // update row (1, 'tom', 1) -> (1, 'tom', 2)
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " (id, status) values (1, 2)");
+            commitWithException(conn);
+
+            // unverified row doesn't match the filter status = 1 but the previous verified
+            // version of the same row matches the filter
+            String selectSql = "SELECT * from " + dataTableName + " WHERE name = 'tom' AND status = 1";
+            // Verify that we will read from the index table
+            assertExplainPlan(conn, selectSql, dataTableName, indexName);
+            try (ResultSet rs = conn.createStatement().executeQuery(selectSql)) {
+                assertTrue(rs.next());
+                assertEquals(1, rs.getInt(1));
+            } catch (AssertionError e) {
+                TestUtil.dumpTable(conn, TableName.valueOf(indexName));
+                throw e;
+            }
+
+            // fail phase 2
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+            // update row (1, 'tom', 1) -> (1, 'tom', 2)
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " (id, status) values (1, 2)");
+            commitWithException(conn);
+            // unverified row matches the filter status = 2 but the previous verified
+            // version of the same row does not match the filter
+            selectSql = "SELECT * from " + dataTableName + " WHERE name = 'tom' AND status = 2";
+            try (ResultSet rs = conn.createStatement().executeQuery(selectSql)) {
+                assertFalse(rs.next());
+            } catch (AssertionError e) {
+                TestUtil.dumpTable(conn, TableName.valueOf(indexName));
+                throw e;
+            }
+
+            // fail phase 2
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+            // new row (3, 'tom', 1)
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " (id, name, status) values (3, 'tom', 1)");
+            commitWithException(conn);
+
+            // Test aggregate query
+            selectSql = "SELECT count(*) from " + dataTableName + " WHERE name = 'tom' and  id > 1";
+            // Verify that we will read from the index table
+            assertExplainPlan(conn, selectSql, dataTableName, indexName);
+            try (ResultSet rs = conn.createStatement().executeQuery(selectSql)) {
+                assertTrue(rs.next());
+                assertEquals(0, rs.getInt(1));
+            } catch (AssertionError e) {
+                TestUtil.dumpTable(conn, TableName.valueOf(indexName));
+                throw e;
+            }
+        }
+    }
+
+    @Test
+    public void testUnverifiedIndexRowWithSkipScanFilter() throws Exception {
+        if (async) {
+            // No need to run the same test twice one for async = true and the other for async = false
+            return;
+        }
+
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            String dataTableName = generateUniqueName();
+            String indexName = generateUniqueName();
+            populateTable(dataTableName); // with two rows ('a', 'ab', 'abc', 'abcd') and ('b', 'bc', 'bcd', 'bcde')
+            conn.createStatement().execute("CREATE INDEX " + indexName + " on " +
+                    dataTableName + " (val1, val2) include (val3)" + this.indexDDLOptions);
+            conn.commit();
+
+            // fail phase 2
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+            // update row ('b', 'bc', 'bcd', 'bcde') -> ('b', 'bc', 'bcdd', 'bcde')
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " (id, val1) values ('b', 'bcc')");
+            commitWithException(conn);
+
+            String selectSql = "SELECT id, val1, val3 from " + dataTableName + " WHERE val1 IN ('ab', 'bcc') ";
+            // Verify that we will read from the index table
+            try (ResultSet rs = conn.createStatement().executeQuery("EXPLAIN " + selectSql)) {
+                String actualExplainPlan = QueryUtil.getExplainPlan(rs);
+                String expectedExplainPlan = String.format("SKIP SCAN ON 2 KEYS OVER %s", indexName);
+                assertTrue(actualExplainPlan.contains(expectedExplainPlan));
+            }
+            try (ResultSet rs = conn.createStatement().executeQuery(selectSql)) {
+                assertTrue(rs.next());
+                assertEquals("a", rs.getString("id"));
+                assertEquals("ab", rs.getString("val1"));
+                assertEquals("abcd", rs.getString("val3"));
+                assertFalse(rs.next());
+            } catch (AssertionError e) {
+                TestUtil.dumpTable(conn, TableName.valueOf(indexName));
+                throw e;
+            }
+
+            // fail phase 2
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+            // update row ('b', 'bc', 'bcd', 'bcde') -> ('b', 'bc', 'bcd', 'bcdf')
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " (id, val3) values ('b', 'bcdf')");
+            commitWithException(conn);
+
+            selectSql = "SELECT id, val3 from " + dataTableName +
+                    " WHERE val1 IN ('bc') AND val2 IN ('bcd', 'xcdf') AND val3 = 'bcde' ";
+            // Verify that we will read from the index table
+            try (ResultSet rs = conn.createStatement().executeQuery("EXPLAIN " + selectSql)) {
+                String actualExplainPlan = QueryUtil.getExplainPlan(rs);
+                String expectedExplainPlan = String.format("SKIP SCAN ON 2 KEYS OVER %s", indexName);
+                String filter = "SERVER FILTER BY";
+                assertTrue(String.format("actual=%s", actualExplainPlan),
+                        actualExplainPlan.contains(expectedExplainPlan));
+                assertTrue(String.format("actual=%s", actualExplainPlan),
+                        actualExplainPlan.contains(filter));
+            }
+            try (ResultSet rs = conn.createStatement().executeQuery(selectSql)) {
+                assertTrue(rs.next());
+                assertEquals("b", rs.getString("id"));
+                assertEquals("bcde", rs.getString("val3"));
+                assertFalse(rs.next());
+            } catch (AssertionError e) {
+                TestUtil.dumpTable(conn, TableName.valueOf(indexName));
+                throw e;
+            }
+        }
+    }
+
+    @Test
+    public void testUnverifiedIndexRowWithFirstKeyOnlyFilter() throws Exception {
+        if (async) {
+            // No need to run the same test twice one for async = true and the other for async = false
+            return;
+        }
+
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            String dataTableName = generateUniqueName();
+            String indexName = generateUniqueName();
+            populateTable(dataTableName); // with two rows ('a', 'ab', 'abc', 'abcd') and ('b', 'bc', 'bcd', 'bcde')
+            conn.createStatement().execute("CREATE INDEX " + indexName + " on " +
+                    dataTableName + " (val1, id, val2, val3) " + this.indexDDLOptions);
+            conn.commit();
+
+            // fail phase 2
+            IndexRegionObserver.setFailDataTableUpdatesForTesting(true);
+            // update row ('b', 'bc', 'bcd', 'bcde') -> ('b', 'bc', 'bcdd', 'bcde')
+            conn.createStatement().execute(
+                    "upsert into " + dataTableName + " (id, val3) values ('b', 'bcdf')");
+            commitWithException(conn);
+
+            String selectSql = "SELECT id, val3 from " + dataTableName + " WHERE val1 = 'bc' and val2 = 'bcd' ";
+            // Verify that we will read from the index table
+            try (ResultSet rs = conn.createStatement().executeQuery("EXPLAIN " + selectSql)) {
+                String actualExplainPlan = QueryUtil.getExplainPlan(rs);
+                String expectedExplainPlan = String.format("RANGE SCAN OVER %s", indexName);
+                String filter = "SERVER FILTER BY FIRST KEY ONLY AND";
+                assertTrue(String.format("actual=%s", actualExplainPlan),
+                        actualExplainPlan.contains(expectedExplainPlan));
+                assertTrue(String.format("actual=%s", actualExplainPlan),
+                        actualExplainPlan.contains(filter));
+            }
+            try (ResultSet rs = conn.createStatement().executeQuery(selectSql)) {
+                assertTrue(rs.next());
+                assertEquals("b", rs.getString("id"));
+                assertEquals("bcde", rs.getString("val3"));
+                assertFalse(rs.next());
+            } catch (AssertionError e) {
+                TestUtil.dumpTable(conn, TableName.valueOf(indexName));
+                throw e;
+            }
+        }
+    }
+
     @Test
     public void testViewIndexRowUpdate() throws Exception {
         if (async) {
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/UnverifiedRowFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/UnverifiedRowFilter.java
new file mode 100644
index 0000000000..400b48d477
--- /dev/null
+++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/UnverifiedRowFilter.java
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.filter;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.filter.Filter;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
+import org.apache.phoenix.util.ScanUtil;
+
+import static org.apache.phoenix.query.QueryConstants.VERIFIED_BYTES;
+
+/**
+ * This filter overrides the behavior of delegate so that we do not jump to
+ * the next row if the row is unverified and doesn't match the filter since
+ * it is possible that a previous verified version of the same row could match
+ * the filter and thus should be included in the results.
+ * For tables using encoded columns, the empty column is the first column the
+ * filter processes, so we can check whether it is verified or not.
+ * If no encoding is used, the empty column is the last column to be processed
+ * by the filter, so we have to wait to determine whether the row is verified
+ * or not.
+ */
+public class UnverifiedRowFilter extends DelegateFilter {
+
+    private final byte[] emptyCF;
+    private final byte[] emptyCQ;
+    private boolean verified = false;
+    // save the code from delegate filter while waiting for the empty column
+    private ReturnCode recordedRetCode = null;
+
+    private void init() {
+        verified = false;
+        recordedRetCode = null;
+    }
+    public UnverifiedRowFilter(Filter delegate, byte[] emptyCF, byte[] emptyCQ) {
+        super(delegate);
+        Preconditions.checkArgument(emptyCF != null,
+                "Column family must not be null");
+        Preconditions.checkArgument(emptyCQ != null,
+                "Column qualifier must not be null");
+        this.emptyCF = emptyCF;
+        this.emptyCQ = emptyCQ;
+        init();
+    }
+
+    @Override
+    public void reset() throws IOException {
+        init();
+        delegate.reset();
+    }
+
+    @Override
+    public ReturnCode filterKeyValue(Cell v) throws IOException {
+        return filterCell(v);
+    }
+
+    @Override
+    public ReturnCode filterCell(final Cell cell) throws IOException {
+        if (verified) {
+            // we have processed the empty column and found that it is verified
+            return delegate.filterCell(cell);
+        }
+
+        if (ScanUtil.isEmptyColumn(cell, emptyCF, emptyCQ)) {
+            verified = Bytes.compareTo(
+                    cell.getValueArray(), cell.getValueOffset(), cell.getValueLength(),
+                    VERIFIED_BYTES, 0, VERIFIED_BYTES.length) == 0;
+            if (verified) {
+                // if we saved the return code while waiting for the empty
+                // column, use that code else call the delegate
+                return recordedRetCode != null ? recordedRetCode : delegate.filterCell(cell);
+            } else {
+                // it is an unverified row, no need to look at more columns
+                // include it so that it can be repaired and evaluated again by the filter
+                return ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW;
+            }
+        }
+        // we haven't seen the empty column yet so don't know whether
+        // the row is verified or not
+
+        if (recordedRetCode != null) {
+            // we already have recorded the return code from the wrapped
+            // delegate filter so skip this column
+            return ReturnCode.NEXT_COL;
+        }
+        ReturnCode ret = delegate.filterCell(cell);
+        if (ret == ReturnCode.NEXT_ROW
+                || ret == ReturnCode.INCLUDE_AND_SEEK_NEXT_ROW) {
+            // Save the return code but don't move to the next row.
+            // Continue processing the current row till we find the empty column
+            recordedRetCode = ret;
+            ret = ReturnCode.NEXT_COL;
+        }
+        return ret;
+    }
+
+    @Override
+    public void filterRowCells(List<Cell> kvs) throws IOException {
+        if (verified) {
+            delegate.filterRowCells(kvs);
+        }
+    }
+
+    @Override
+    public boolean filterRow() throws IOException {
+        if (verified) {
+            return delegate.filterRow();
+        }
+        return false;
+    }
+}
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 24cb22c87d..1d2a004d39 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
@@ -49,6 +49,7 @@ import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
 import org.apache.hadoop.hbase.coprocessor.RegionObserver;
 import org.apache.hadoop.hbase.filter.Filter;
 import org.apache.hadoop.hbase.filter.FilterList;
+import org.apache.hadoop.hbase.filter.FirstKeyOnlyFilter;
 import org.apache.hadoop.hbase.filter.PageFilter;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.regionserver.Region;
@@ -58,6 +59,7 @@ import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.phoenix.coprocessor.BaseRegionScanner;
 import org.apache.phoenix.coprocessor.BaseScannerRegionObserver;
 import org.apache.phoenix.filter.PagedFilter;
+import org.apache.phoenix.filter.UnverifiedRowFilter;
 import org.apache.phoenix.hbase.index.covered.update.ColumnReference;
 import org.apache.phoenix.hbase.index.metrics.GlobalIndexCheckerSource;
 import org.apache.phoenix.hbase.index.metrics.MetricsIndexerSourceFactory;
@@ -142,12 +144,12 @@ public class GlobalIndexChecker extends BaseScannerRegionObserver implements Reg
         private GlobalIndexCheckerSource metricsSource;
         private long rowCount = 0;
         private long pageSize = Long.MAX_VALUE;
-        private boolean restartScanDueToPageFilterRemoval = false;
         private boolean hasMore;
         private double loggingPercent;
         private Random random;
         private String indexName;
         private long pageSizeMs;
+        private boolean initialized = false;
 
         public GlobalIndexScanner(RegionCoprocessorEnvironment env,
                                   Scan scan,
@@ -193,8 +195,62 @@ public class GlobalIndexChecker extends BaseScannerRegionObserver implements Reg
             return scanner.getMaxResultSize();
         }
 
+        private void init() throws IOException {
+            PageFilter pageFilter = removePageFilter(scan);
+            if (pageFilter != null) {
+                pageSize = pageFilter.getPageSize();
+                scanner.close();
+                scanner = ((BaseRegionScanner)delegate).getNewRegionScanner(scan);
+            }
+            else {
+                pageSize = Long.MAX_VALUE;
+            }
+
+            Filter filter = scan.getFilter();
+            Filter delegateFilter = filter;
+            if (filter instanceof PagedFilter) {
+                delegateFilter = ((PagedFilter) filter).getDelegateFilter();
+            }
+            if (shouldCreateUnverifiedRowFilter(delegateFilter)) {
+                // we need to ensure that the PagingFilter remains the
+                // topmost (or outermost) filter so wrap the UnverifiedRowFilter
+                // around the original delegate and then set the UnverifiedRowFilter
+                // as the delegate of the PagingFilter
+                UnverifiedRowFilter unverifiedRowFilter =
+                        new UnverifiedRowFilter(delegateFilter, emptyCF, emptyCQ);
+                if (filter instanceof PagedFilter) {
+                    ((PagedFilter) filter).setDelegateFilter(unverifiedRowFilter);
+                } else {
+                    scan.setFilter(unverifiedRowFilter);
+                }
+                scanner.close();
+                scanner = ((BaseRegionScanner) delegate).getNewRegionScanner(scan);
+            }
+        }
+
+        private boolean shouldCreateUnverifiedRowFilter(Filter delegateFilter) {
+            if (delegateFilter == null) {
+                return false;
+            }
+            Filter wrappedFilter = delegateFilter;
+            if (delegateFilter instanceof FilterList) {
+                List<Filter> filters = ((FilterList) delegateFilter).getFilters();
+                wrappedFilter = filters.get(0);
+            }
+            // Optimization since FirstKeyOnlyFilter and EmptyColumnOnlyFilter
+            // always include the empty column in the scan result
+            if (wrappedFilter instanceof FirstKeyOnlyFilter) {
+                return false;
+            }
+            return true;
+        }
+
         public boolean next(List<Cell> result, boolean raw) throws IOException {
             try {
+                if (!initialized) {
+                    init();
+                    initialized = true;
+                }
                 long startTime = EnvironmentEdgeManager.currentTimeMillis();
                 do {
                     if (raw) {
@@ -331,11 +387,6 @@ public class GlobalIndexChecker extends BaseScannerRegionObserver implements Reg
 
         private void repairIndexRows(byte[] indexRowKey, long ts, List<Cell> row) throws IOException {
             if (buildIndexScan == null) {
-                PageFilter pageFilter = removePageFilter(scan);
-                if (pageFilter != null) {
-                    pageSize = pageFilter.getPageSize();
-                    restartScanDueToPageFilterRemoval = true;
-                }
                 buildIndexScan = new Scan();
                 indexScan = new Scan(scan);
                 singleRowIndexScan = new Scan(scan);
@@ -384,20 +435,10 @@ public class GlobalIndexChecker extends BaseScannerRegionObserver implements Reg
                 // Skip this unverified row (i.e., do not return it to the client). Just retuning empty row is
                 // sufficient to do that
                 row.clear();
-                if (restartScanDueToPageFilterRemoval) {
-                    scanner.close();
-                    indexScan.withStartRow(indexRowKey, false);
-                    scanner = ((BaseRegionScanner)delegate).getNewRegionScanner(indexScan);
-                    hasMore = true;
-                    // Set restartScanDueToPageFilterRemoval to false as we do not restart the scan unnecessarily next time
-                    restartScanDueToPageFilterRemoval = false;
-                }
                 return;
             }
             // An index row has been built. Close the current scanner as the newly built row will not be visible to it
             scanner.close();
-            // Set restartScanDueToPageFilterRemoval to false as we do not restart the scan unnecessarily next time
-            restartScanDueToPageFilterRemoval = false;
             if (code == RebuildReturnCode.NO_INDEX_ROW.getValue()) {
                 // This means there exists a data table row for the data row key derived from this unverified index row
                 // but the data table row does not point back to the index row.
@@ -465,11 +506,8 @@ public class GlobalIndexChecker extends BaseScannerRegionObserver implements Reg
                 singleRowScanner.next(row);
                 singleRowScanner.close();
                 if (row.isEmpty()) {
-                    LOG.error("Could not find the newly rebuilt index row with row key " +
-                            Bytes.toStringBinary(indexRowKey) + " for table " +
-                            region.getRegionInfo().getTable().getNameAsString());
-                    // This was not expected. The new build index row must be deleted before opening the new scanner
-                    // possibly by compaction
+                    // This can happen if the unverified row matches the filter
+                    // but after repair the verified row doesn't match the filter
                     return;
                 }
                 if (isDummy(row)) {