This is an automated email from the ASF dual-hosted git repository.
gjacoby pushed a commit to branch 4.16
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.16 by this push:
new 9b096bd PHOENIX-6437: Parent-Child Delete marker should get
replicated via Sy… (#1219)
9b096bd is described below
commit 9b096bd590325eb41ebf60d2d41649823847044a
Author: ankitjain64 <[email protected]>
AuthorDate: Wed May 5 16:44:45 2021 -0700
PHOENIX-6437: Parent-Child Delete marker should get replicated via Sy…
(#1219)
* PHOENIX-6437: Parent-Child Delete marker should get replicated via
SystemCatalogWALEntryFilter
Co-authored-by: Ankit Jain <[email protected]>
---
.../replication/SystemCatalogWALEntryFilterIT.java | 194 +++++++++++++++++----
.../replication/SystemCatalogWALEntryFilter.java | 19 +-
2 files changed, 173 insertions(+), 40 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 ab6af25..24d4990 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
@@ -22,12 +22,14 @@ import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.*;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.regionserver.wal.HLogKey;
import org.apache.hadoop.hbase.replication.ChainWALEntryFilter;
import org.apache.hadoop.hbase.regionserver.wal.WALEdit;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.wal.WAL;
import org.apache.hadoop.hbase.wal.WALKey;
import org.apache.phoenix.end2end.ParallelStatsDisabledIT;
+import org.apache.phoenix.hbase.index.wal.IndexedKeyValue;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.mapreduce.util.ConnectionUtil;
import org.apache.phoenix.schema.PTable;
@@ -67,8 +69,8 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
+ NONTENANT_VIEW_NAME + "(" + VIEW_COLUMN_NAME + " varchar) AS SELECT *
FROM "
+ TestUtil.ENTITY_HISTORY_TABLE_NAME + " WHERE OLD_VALUE like 'E%'";
- private static final String DROP_TENANT_VIEW_SQL = "DROP VIEW IF EXISTS " +
TENANT_VIEW_NAME;
- private static final String DROP_NONTENANT_VIEW_SQL = "DROP VIEW IF EXISTS "
+ NONTENANT_VIEW_NAME;
+ private static final String DROP_TENANT_VIEW_SQL = "DROP VIEW IF EXISTS " +
SCHEMA_NAME + "." + TENANT_VIEW_NAME;
+ private static final String DROP_NONTENANT_VIEW_SQL = "DROP VIEW IF EXISTS "
+ SCHEMA_NAME + "." + NONTENANT_VIEW_NAME;
private static PTable catalogTable;
private static PTable childLinkTable;
private static WALKey walKeyCatalog = null;
@@ -97,24 +99,13 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
PhoenixDatabaseMetaData.SYSTEM_CHILD_LINK_NAME), 0, 0, uuid);
};
Assert.assertNotNull(catalogTable);
- try (java.sql.Connection connection =
-
ConnectionUtil.getInputConnection(getUtility().getConfiguration(), new
Properties())) {
- connection.createStatement().execute(CREATE_NONTENANT_VIEW_SQL);
- };
+ createNonTenantView();
}
@AfterClass
public static synchronized void tearDown() throws Exception {
- Properties tenantProperties = new Properties();
- tenantProperties.setProperty("TenantId", TENANT_ID);
- try (java.sql.Connection connection =
-
ConnectionUtil.getInputConnection(getUtility().getConfiguration(),
tenantProperties)) {
- connection.createStatement().execute(DROP_TENANT_VIEW_SQL);
- }
- try (java.sql.Connection connection =
-
ConnectionUtil.getInputConnection(getUtility().getConfiguration(), new
Properties())) {
- connection.createStatement().execute(DROP_NONTENANT_VIEW_SQL);
- }
+ dropTenantView();
+ dropNonTenantView();
}
@Test
@@ -136,7 +127,7 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
WAL.Entry nonTenantEntryCatalog = getEntry(systemCatalogTableName,
nonTenantGetCatalog);
WAL.Entry tenantEntryCatalog = getEntry(systemCatalogTableName,
tenantGetCatalog);
- int tenantRowCount = getAndAssertTenantCountInEdit(tenantEntryCatalog);
+ int tenantRowCount = getAndAssertCountInEdit(tenantEntryCatalog, true);
Assert.assertTrue(tenantRowCount > 0);
//verify that the tenant view WAL.Entry passes the filter and the
non-tenant view does not
@@ -151,9 +142,9 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
WAL.Entry filteredTenantEntryCatalog =
chainWALEntryFilter.filter(tenantEntryCatalog);
Assert.assertNotNull("Tenant view was filtered when it shouldn't be!",
- filteredTenantEntryCatalog);
+ filteredTenantEntryCatalog);
Assert.assertEquals("Not all data for replicated for tenant",
tenantRowCount,
- getAndAssertTenantCountInEdit(filteredTenantEntryCatalog));
+ getAndAssertCountInEdit(filteredTenantEntryCatalog, true));
//now check that a WAL.Entry with cells from both a tenant and a non-tenant
//catalog row only allow the tenant cells through
@@ -179,7 +170,7 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
WAL.Entry tenantEntryChildLink = getEntry(systemChildLinkTableName,
tenantGetChildLink);
WAL.Entry nonTenantEntryChildLink = getEntry(systemChildLinkTableName,
nonTenantGetChildLink);
- int tenantRowCount = getAndAssertTenantCountInEdit(tenantEntryChildLink);
+ int tenantRowCount = getAndAssertCountInEdit(tenantEntryChildLink, true);
Assert.assertTrue(tenantRowCount > 0);
//verify that the tenant view WAL.Entry passes the filter and the
non-tenant view does not
@@ -190,13 +181,13 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
Assert.assertTrue(nonTenantEntryChildLink.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 child link should not get
filtered",
- chainWALEntryFilter.filter(nonTenantEntryChildLink).getEdit().isEmpty());
+
chainWALEntryFilter.filter(nonTenantEntryChildLink).getEdit().isEmpty());
WAL.Entry filteredTenantEntryChildLink =
chainWALEntryFilter.filter(tenantEntryChildLink);
Assert.assertNotNull("Tenant view was filtered when it shouldn't be!",
- filteredTenantEntryChildLink);
+ filteredTenantEntryChildLink);
Assert.assertEquals("Not all data for replicated for tenant",
tenantRowCount,
- getAndAssertTenantCountInEdit(filteredTenantEntryChildLink));
+ getAndAssertCountInEdit(filteredTenantEntryChildLink, true));
//now check that a WAL.Entry with cells from both a tenant and a non-tenant
// child link row only allow the tenant cells through
@@ -206,12 +197,54 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
WAL.Entry comboEntry = new WAL.Entry(walKeyChildLink, comboEdit);
Assert.assertEquals(tenantEntryChildLink.getEdit().size() +
nonTenantEntryChildLink.getEdit().size()
- , comboEntry.getEdit().size());
+ , comboEntry.getEdit().size());
Assert.assertEquals(tenantEntryChildLink.getEdit().size(),
- chainWALEntryFilter.filter(comboEntry).getEdit().size());
+ chainWALEntryFilter.filter(comboEntry).getEdit().size());
+ }
+
+ /**
+ * Validates the behavior for parent-child link's delete marker via
SystemCatalogWalEntryFilter.
+ * 1. Filtered for non-tenant views.
+ * 2. Not filtered for tenant views.
+ * */
+ @Test
+ public void testDeleteMarkerForParentChildLink() throws Exception{
+ // Since for 4.16+ all parent-child links are stored in SYSTEM.CHILD_LINK,
only
+ // checking for that table in this test.
+
+ // Make sure link row exists.
+ WAL.Entry childLinkEntry = getEntry(systemChildLinkTableName, new Scan(),
+ false);
+ int tenantRowCount = getAndAssertCountInEdit(childLinkEntry, true);
+ int nonTenantRowCount = getAndAssertCountInEdit(childLinkEntry, false);
+ Assert.assertTrue(tenantRowCount > 0 && nonTenantRowCount > 0 );
+
+ // Drop both tenant and non-tenant view.
+ dropTenantView();
+ dropNonTenantView();
+
+ // Delete Marker for non-tenant view should get filtered and for
tenant-view it should not.
+ SystemCatalogWALEntryFilter filter = new SystemCatalogWALEntryFilter();
+ // Chain the system catalog WAL entry filter to ChainWALEntryFilter
+ ChainWALEntryFilter chainWALEntryFilter = new ChainWALEntryFilter(filter);
+ childLinkEntry = getEntry(systemChildLinkTableName, new Scan(),
+ false);
+ int tenantDeleteCountBeforeFilter =
getDeleteFamilyCellCountInEntry(childLinkEntry, true);
+ int nonTenantDeleteCountBeforeFilter =
getDeleteFamilyCellCountInEntry(childLinkEntry, false);
+ // Make sure both tenant and non-tenant delete marker exists before
filtering
+ Assert.assertTrue(tenantDeleteCountBeforeFilter > 0 &&
nonTenantDeleteCountBeforeFilter > 0 );
+
+ WAL.Entry filteredEntry = chainWALEntryFilter.filter(childLinkEntry);
+ int tenantDeleteCountAfterFilter =
getDeleteFamilyCellCountInEntry(filteredEntry, true);
+ int nonTenantDeleteCountAfterFilter =
getDeleteFamilyCellCountInEntry(filteredEntry, false);
+ Assert.assertTrue(tenantDeleteCountAfterFilter ==
tenantDeleteCountBeforeFilter && nonTenantDeleteCountAfterFilter == 0 );
+
+ // setup views again.
+ createTenantView();
+ createNonTenantView();
}
- public Get getGet(PTable catalogTable, byte[] tenantId, String viewName) {
+ private Get getGet(PTable catalogTable, byte[] tenantId, String viewName) {
byte[][] tenantKeyParts = new byte[5][];
tenantKeyParts[0] = tenantId;
tenantKeyParts[1] = Bytes.toBytes(SCHEMA_NAME.toUpperCase());
@@ -225,7 +258,7 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
return new Get(key.copyBytes());
}
- public Get getGetChildLink(PTable catalogTable, byte[] tenantId, String
viewName) {
+ private Get getGetChildLink(PTable catalogTable, byte[] tenantId, String
viewName) {
byte[][] tenantKeyParts = new byte[5][];
tenantKeyParts[0] = ByteUtil.EMPTY_BYTE_ARRAY;
tenantKeyParts[1] = ByteUtil.EMPTY_BYTE_ARRAY;
@@ -246,21 +279,54 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
boolean isChildLinkForTenantId = row.contains(tenantId)
&& CellUtil.matchingQualifier(cell,
PhoenixDatabaseMetaData.LINK_TYPE_BYTES);
- return isTenantIdLeading || isChildLinkForTenantId;
+ boolean isDeleteMarkerForLinkRow = row.contains(tenantId) &&
CellUtil.isDeleteFamily(cell);
+ return isTenantIdLeading || isChildLinkForTenantId ||
isDeleteMarkerForLinkRow;
}
- private int getAndAssertTenantCountInEdit(WAL.Entry entry) {
- int count = 0;
+ /**
+ * Asserts and returns cell count in the WAL.Entry. if tenantOwned is true,
tenant owned cell count is
+ * returned else non-tenant cell count.
+ * @Param entry {@link WAL.Entry}
+ * @Param tenantOwned {@link Boolean}
+ * */
+ private int getAndAssertCountInEdit(WAL.Entry entry, boolean tenantOwned) {
+ int tenantCount = 0;
+ int nonTenantCount = 0;
for (Cell cell : entry.getEdit().getCells()) {
if (isTenantOwnedCell(cell, TENANT_ID)) {
- count = count + 1;
+ tenantCount = tenantCount + 1;
+ } else {
+ nonTenantCount = nonTenantCount + 1;
}
}
+ int count = tenantOwned ? tenantCount : nonTenantCount;
Assert.assertTrue(count > 0);
return count;
}
- public WAL.Entry getEntry(TableName tableName, Get get) throws IOException {
+ /**
+ * Returns delete family cell count in the WAL.Entry. if tenantOwned is
true, tenant owned cell count is
+ * returned else non-tenant cell count.
+ * @Param entry {@link WAL.Entry}
+ * @Param tenantOwned {@link Boolean}
+ * */
+ private int getDeleteFamilyCellCountInEntry(WAL.Entry entry, boolean
tenantOwned) {
+ int tenantCount = 0;
+ int nonTenantCount = 0;
+ for (Cell cell : entry.getEdit().getCells()) {
+ if (CellUtil.isDeleteFamily(cell)) {
+ if (isTenantOwnedCell(cell, TENANT_ID)) {
+ tenantCount = tenantCount + 1;
+ } else {
+ nonTenantCount = nonTenantCount + 1;
+ }
+ }
+ }
+ return tenantOwned ? tenantCount : nonTenantCount;
+ }
+
+
+ private WAL.Entry getEntry(TableName tableName, Get get) throws IOException {
WAL.Entry entry = null;
try(Connection conn =
ConnectionFactory.createConnection(getUtility().getConfiguration())){
Table htable = conn.getTable(tableName);
@@ -281,4 +347,68 @@ public class SystemCatalogWALEntryFilterIT extends
ParallelStatsDisabledIT {
}
return entry;
}
+
+ private WAL.Entry getEntry(TableName tableName, Scan scan, boolean
addIndexedKeyValueCell)
+ throws IOException {
+ WAL.Entry entry = null;
+ try(HConnection conn =
HConnectionManager.createConnection(getUtility().getConfiguration())) {
+ HTableInterface htable = conn.getTable(tableName);
+ scan.setRaw(true);
+ ResultScanner scanner = htable.getScanner(scan);
+ WALEdit edit = new WALEdit();
+ if (addIndexedKeyValueCell) {
+ // add IndexedKeyValue type cell as the first cell
+ edit.add(new IndexedKeyValue());
+ }
+
+ for (Result r : scanner) {
+ if (r != null) {
+ List<Cell> cellList = r.listCells();
+ for (Cell c : cellList) {
+ edit.add(c);
+ }
+ }
+ }
+ Assert.assertFalse("No WALEdits were loaded!", edit.isEmpty());
+ HLogKey key = new HLogKey(REGION, tableName, 0, 0, uuid);
+ entry = new WAL.Entry(key, edit);
+ }
+ return entry;
+ }
+
+ private static void dropTenantView() throws Exception {
+ Properties tenantProperties = new Properties();
+ tenantProperties.setProperty("TenantId", TENANT_ID);
+ try (java.sql.Connection connection =
+ ConnectionUtil.getInputConnection(getUtility().getConfiguration(),
tenantProperties)) {
+ connection.createStatement().execute(DROP_TENANT_VIEW_SQL);
+ connection.commit();
+ }
+ }
+
+ private static void dropNonTenantView() throws Exception {
+ try (java.sql.Connection connection =
+ ConnectionUtil.getInputConnection(getUtility().getConfiguration(), new
Properties())) {
+
+ connection.createStatement().execute(DROP_NONTENANT_VIEW_SQL);
+ }
+ }
+
+ private static void createTenantView() throws Exception {
+ Properties tenantProperties = new Properties();
+ tenantProperties.setProperty("TenantId", TENANT_ID);
+ try (java.sql.Connection connection =
+ ConnectionUtil.getInputConnection(getUtility().getConfiguration(),
tenantProperties)) {
+ connection.createStatement().execute(CREATE_TENANT_VIEW_SQL);
+ connection.commit();
+ }
+ }
+
+ private static void createNonTenantView() throws Exception {
+ try (java.sql.Connection connection =
+ ConnectionUtil.getInputConnection(getUtility().getConfiguration(), new
Properties())) {
+ connection.createStatement().execute(CREATE_NONTENANT_VIEW_SQL);
+ connection.commit();
+ }
+ }
}
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 3f29e8e..874d810 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
@@ -90,7 +90,8 @@ public class SystemCatalogWALEntryFilter implements
* tenant id, system.child_link table have tenant owned data for parent child
* links. In this case, the column qualifier is
* {@code PhoenixDatabaseMetaData#LINK_TYPE_BYTES} and value is
- * {@code PTable.LinkType.CHILD_TABLE}.
+ * {@code PTable.LinkType.CHILD_TABLE}. For corresponding delete markers the
+ * KeyValue type {@code KeyValue.Type} is {@code KeyValue.Type.DeleteFamily}
* @param cell hbase cell
* @return true if the cell is tenant owned
*/
@@ -103,18 +104,20 @@ public class SystemCatalogWALEntryFilter implements
if (!isTenantRowCell) {
boolean isChildLink = CellUtil.matchingQualifier(
cell, PhoenixDatabaseMetaData.LINK_TYPE_BYTES);
- if (isChildLink) {
- if (CellUtil.matchingValue(cell, CHILD_TABLE_BYTES)) {
+ // Check if cell is of type LINK_TYPE with value 4 or DeleteFamily
+ if ((isChildLink && CellUtil.matchingValue(cell, CHILD_TABLE_BYTES)) ||
+ CellUtil.isDeleteFamily(cell) ) {
byte[][] rowViewKeyMetadata = new byte[NUM_COLUMNS_PRIMARY_KEY][];
SchemaUtil.getVarChars(key.get(), key.getOffset(),
key.getLength(), 0, rowViewKeyMetadata);
- // if the child link is to a tenant-owned view,
- // the COLUMN_NAME field will be the byte[] of the tenant
- //otherwise, it will be an empty byte array
- // (NOT QueryConstants.SEPARATOR_BYTE, but a byte[0])
+ /** if the child link is to a tenant-owned view, the COLUMN_NAME
field will be
+ * the byte[] of the tenant otherwise, it will be an empty byte array
+ * (NOT QueryConstants.SEPARATOR_BYTE, but a byte[0]). This
assumption is also
+ * true for child link's delete markers in SYSTEM.CHILD_LINK as it
only contains link
+ * rows and does not deal with other type of rows like column rows
that also has
+ * COLUMN_NAME populated with actual column name.**/
isChildLinkToTenantView =
rowViewKeyMetadata[COLUMN_NAME_INDEX].length != 0;
- }
}
}
return isTenantRowCell || isChildLinkToTenantView;
