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 <sandeep....@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; } }