[phoenix] branch master updated: PHOENIX-5895 Leverage WALCellFilter in the SystemCatalogWALEntryFilter to replicate system catalog table

[gj...@apache.org]

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

gjacoby 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 6cc9d50  PHOENIX-5895 Leverage WALCellFilter in the SystemCatalogWALEntryFilter to replicate system catalog table
6cc9d50 is described below

commit 6cc9d5030da355abc6c8ab9eac9cc32097113e17
Author: Sandeep Pal <sa...@salesforce.com>
AuthorDate: Sun Nov 1 22:38:24 2020 -0800

    PHOENIX-5895 Leverage WALCellFilter in the SystemCatalogWALEntryFilter to replicate system catalog table
---
 .../replication/SystemCatalogWALEntryFilterIT.java | 17 +++++---
 .../replication/SystemCatalogWALEntryFilter.java   | 51 +++++++++++-----------
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilterIT.java b/phoenix-core/src/it/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilterIT.java
index df4d97c..4a704a6 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilterIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilterIT.java
@@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.Table;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.replication.ChainWALEntryFilter;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.wal.WAL;
 import org.apache.hadoop.hbase.wal.WALEdit;
@@ -134,11 +135,17 @@ public class SystemCatalogWALEntryFilterIT extends ParallelStatsDisabledIT {
 
     //verify that the tenant view WAL.Entry passes the filter and the non-tenant view does not
     SystemCatalogWALEntryFilter filter = new SystemCatalogWALEntryFilter();
-    Assert.assertNull(filter.filter(nonTenantEntry));
-    WAL.Entry filteredTenantEntry = filter.filter(tenantEntry);
+    // Chain the system catalog WAL entry filter to ChainWALEntryFilter
+    ChainWALEntryFilter chainWALEntryFilter = new ChainWALEntryFilter(filter);
+    // Asserting the WALEdit for non tenant has cells before getting filtered
+    Assert.assertTrue(nonTenantEntry.getEdit().size() > 0);
+    // All the cells will get removed by the filter since they do not belong to tenant
+    Assert.assertTrue("Non tenant edits for system catalog should not get filtered",
+        chainWALEntryFilter.filter(nonTenantEntry).getEdit().isEmpty());
+    WAL.Entry filteredTenantEntry = chainWALEntryFilter.filter(tenantEntry);
     Assert.assertNotNull("Tenant view was filtered when it shouldn't be!", filteredTenantEntry);
-    Assert.assertEquals(tenantEntry.getEdit().size(),
-        filter.filter(tenantEntry).getEdit().size());
+    Assert.assertEquals("filtered entry is not correct",
+        tenantEntry.getEdit().size(), filteredTenantEntry.getEdit().size());
 
     //now check that a WAL.Entry with cells from both a tenant and a non-tenant
     //catalog row only allow the tenant cells through
@@ -150,7 +157,7 @@ public class SystemCatalogWALEntryFilterIT extends ParallelStatsDisabledIT {
     Assert.assertEquals(tenantEntry.getEdit().size() + nonTenantEntry.getEdit().size()
         , comboEntry.getEdit().size());
     Assert.assertEquals(tenantEntry.getEdit().size(),
-        filter.filter(comboEntry).getEdit().size());
+        chainWALEntryFilter.filter(comboEntry).getEdit().size());
   }
 
   public Get getGet(PTable catalogTable, byte[] tenantId, String viewName) {
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilter.java
index 6e4c532..1f11807 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilter.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/replication/SystemCatalogWALEntryFilter.java
@@ -17,16 +17,13 @@
  */
 package org.apache.phoenix.replication;
 
-import java.util.List;
-
 import org.apache.hadoop.hbase.Cell;
-import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.replication.WALCellFilter;
 import org.apache.hadoop.hbase.replication.WALEntryFilter;
 import org.apache.hadoop.hbase.wal.WAL;
 import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.util.SchemaUtil;
 
-import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
 
 /**
  * Standard replication of the SYSTEM.CATALOG table can be dangerous because schemas
@@ -35,36 +32,40 @@ import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
  * be copied. This WALEntryFilter will only allow tenant-owned rows in SYSTEM.CATALOG to
  * be replicated. Data from all other tables is automatically passed.
  */
-public class SystemCatalogWALEntryFilter implements WALEntryFilter {
+public class SystemCatalogWALEntryFilter implements
+    WALEntryFilter, WALCellFilter {
+  /**
+   * This is an optimization to just skip the cell filter if we do not care
+   * about cell filter for certain WALEdits.
+   */
+  private boolean skipCellFilter;
 
   @Override
   public WAL.Entry filter(WAL.Entry entry) {
-
-    //if the WAL.Entry's table isn't System.Catalog or System.Child_Link, it auto-passes this filter
-    //TODO: when Phoenix drops support for pre-1.3 versions of HBase, redo as a WALCellFilter
+    // We use the WALCellFilter to filter the cells from entry, WALEntryFilter
+    // should not block anything
+    // if the WAL.Entry's table isn't System.Catalog or System.Child_Link,
+    // it auto-passes this filter
     if (!SchemaUtil.isMetaTable(entry.getKey().getTableName().getName())){
-      return entry;
+      skipCellFilter = true;
+    } else {
+      skipCellFilter = false;
     }
+    return entry;
+  }
 
-    List<Cell> cells = entry.getEdit().getCells();
-    List<Cell> cellsToRemove = Lists.newArrayList();
-    for (Cell cell : cells) {
-      if (!isTenantRowCell(cell)){
-        cellsToRemove.add(cell);
-      }
-    }
-    cells.removeAll(cellsToRemove);
-    if (cells.size() > 0) {
-      return entry;
-    } else {
-      return null;
+  @Override
+  public Cell filterCell(final WAL.Entry entry, final Cell cell) {
+    if (skipCellFilter) {
+      return cell;
     }
+    return isTenantRowCell(cell) ? cell : null;
   }
 
   private boolean isTenantRowCell(Cell cell) {
-    ImmutableBytesWritable key =
-        new ImmutableBytesWritable(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength());
-    //rows in system.catalog that aren't tenant-owned will have a leading separator byte
-    return key.get()[key.getOffset()] != QueryConstants.SEPARATOR_BYTE;
+    // rows in system.catalog that aren't tenant-owned 
+    // will have a leading separator byte
+    return cell.getRowArray()[cell.getRowOffset()]
+        != QueryConstants.SEPARATOR_BYTE;
   }
 }