You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by ChinmaySKulkarni <gi...@git.apache.org> on 2018/09/11 21:46:47 UTC
[GitHub] phoenix pull request #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICAT...
GitHub user ChinmaySKulkarni opened a pull request:
https://github.com/apache/phoenix/pull/346
PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCOPE, and TTL properties stay in sync between the physical data table and index tables
Changes in this commit:
- Disallow specifying any of these properties per-CF when creating any table
- Disallow specifying any of these properties when creating an index
- When creating an index, make sure the above properties are got from the base table
- Disallow specifying CF with these properties when altering a base table
- If any of these properties is altered on a base table, alter this property for all column indexes for that table, as well as all indexes and the view index table for that base table
- If adding a new CF, ensure that it gets these property values (like we used to do for TTL)
- Disallow altering any of these properties for an index
- Add upgrade utility to sync these properties for each base table and its indexes and view index table
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ChinmaySKulkarni/phoenix PHOENIX-3955
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/phoenix/pull/346.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #346
----
commit 3f5bbca6c83ad65d7d57a72a67648147f7cce9e1
Author: Chinmay Kulkarni <ch...@...>
Date: 2018-09-11T21:30:01Z
PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCOPE, and TTL properties stay in sync between the physical data table and index tables
----
---
[GitHub] phoenix pull request #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICAT...
Posted by ChinmaySKulkarni <gi...@git.apache.org>.
Github user ChinmaySKulkarni commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/346#discussion_r220352602
--- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/PropertiesInSyncIT.java ---
@@ -0,0 +1,462 @@
+/*
+ * 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.end2end;
+
+import org.apache.hadoop.hbase.KeepDeletedCells;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.schema.PTable;
+import org.apache.phoenix.util.MetaDataUtil;
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.MetaDataUtil.PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES;
+import static org.apache.phoenix.util.MetaDataUtil.VIEW_INDEX_TABLE_PREFIX;
+import static org.apache.phoenix.util.UpgradeUtil.syncTableAndIndexProperties;
+
+/**
+ * Test properties that need to be kept in sync amongst all column families and indexes of a table
+ */
+public class PropertiesInSyncIT extends ParallelStatsDisabledIT {
+ private static final String COL_FAM1 = "CF1";
+ private static final String COL_FAM2 = "CF2";
+ private static final String NEW_CF = "NEW_CF";
+ private static final String DUMMY_PROP_VALUE = "dummy";
+ private static final int INITIAL_TTL_VALUE = 700;
+ private static final KeepDeletedCells INITIAL_KEEP_DELETED_CELLS_VALUE = KeepDeletedCells.TRUE;
+ private static final int INITIAL_REPLICATION_SCOPE_VALUE = 1;
+ private static final int MODIFIED_TTL_VALUE = INITIAL_TTL_VALUE + 300;
+ private static final KeepDeletedCells MODIFIED_KEEP_DELETED_CELLS_VALUE =
+ (INITIAL_KEEP_DELETED_CELLS_VALUE == KeepDeletedCells.TRUE) ? KeepDeletedCells.FALSE: KeepDeletedCells.TRUE;
+ private static final int MODIFIED_REPLICATION_SCOPE_VALUE = (INITIAL_REPLICATION_SCOPE_VALUE == 1) ? 0 : 1;
+
+
+ // Test that we disallow specifying synced properties to be set per column family when creating a table
+ @Test
+ public void testDisallowSyncedPropsToBeSetColFamSpecificCreateTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = generateUniqueName();
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("create table " + tableName
+ + " (id INTEGER not null primary key, "
+ + COL_FAM1 + ".name varchar(10), " + COL_FAM2 + ".flag boolean) "
+ + COL_FAM1 + "." + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a specific column family");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a specific column family",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+ conn.close();
+ }
+
+ // Test that all column families have the same value of synced properties when creating a table
+ @Test
+ public void testSyncedPropsAllColFamsCreateTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ verifyHBaseColumnFamilyProperties(tableName, conn, false, false);
+ conn.close();
+ }
+
+ // Test that we disallow specifying synced properties to be set when creating an index on a physical table or a view
+ @Test
+ public void testDisallowSyncedPropsToBeSetCreateIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ String localIndexName = tableName + "_LOCAL_IDX";
+ String globalIndexName = tableName + "_GLOBAL_IDX";
+ String viewName = "VIEW_" + tableName;
+ conn.createStatement().execute("create view " + viewName
+ + " (new_col SMALLINT) as select * from " + tableName + " where id > 1");
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("create local index " + localIndexName
+ + " on " + tableName + "(name) "
+ + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a local index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a local index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ try {
+ conn.createStatement().execute("create index " + globalIndexName
+ + " on " + tableName + "(flag) "
+ + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a global index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a global index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ try {
+ conn.createStatement().execute("create index view_index"
+ + " on " + viewName + " (flag)" + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a view index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a view index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+ conn.close();
+ }
+
+ // Test that indexes have the same value of synced properties as their base table
+ @Test
+ public void testSyncedPropsBaseTableCreateIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ createIndexTable(conn, tableName, PTable.IndexType.LOCAL);
+ String globalIndexName = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL);
+
+ // We pass the base table as the physical HBase table since our check includes checking the local index column family too
+ verifyHBaseColumnFamilyProperties(tableName, conn, false, false);
+ verifyHBaseColumnFamilyProperties(globalIndexName, conn, false, false);
+ conn.close();
+ }
+
+ // Test that the physical view index table has the same value of synced properties as its base table
+ @Test
+ public void testSyncedPropsBaseTableCreateViewIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ String viewIndexName = createIndexTable(conn, tableName, null);
+
+ verifyHBaseColumnFamilyProperties(tableName, conn, false, false);
+ verifyHBaseColumnFamilyProperties(viewIndexName, conn, false, false);
+ conn.close();
+ }
+
+ // Test that we disallow specifying synced properties to be set per column family when altering a table
+ @Test
+ public void testDisallowSyncedPropsToBeSetColFamSpecificAlterTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ StringBuilder alterAllSyncedPropsString = new StringBuilder();
+ String modPropString = COL_FAM1 + ".%s=" + DUMMY_PROP_VALUE + ",";
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("alter table " + tableName
+ + " set " + COL_FAM1 + "." + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when altering synced property for a specific column family");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced property for a specific column family",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ alterAllSyncedPropsString.append(String.format(modPropString, propName));
+ }
+
+ // Test the same when we try to set all of these properties at once
+ try {
+ conn.createStatement().execute("alter table " + tableName + " set "
+ + alterAllSyncedPropsString.substring(0, alterAllSyncedPropsString.length() - 1));
+ fail("Should fail with SQLException when altering synced properties for a specific column family");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced properties for a specific column family",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ conn.close();
+ }
+
+ // Test that any alteration of the synced properties gets propagated to all indexes and the physical view index table
+ @Test
+ public void testAlterSyncedPropsPropagateToAllIndexesAndViewIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ Set<String> tablesToCheck = new HashSet<>();
+ tablesToCheck.add(tableName);
+ for (int i=0; i<2; i++) {
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.LOCAL));
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.GLOBAL));
+ }
+ // Create a view and view index
+ tablesToCheck.add(createIndexTable(conn, tableName, null));
+
+ // Now alter the base table's properties. This should get propagated to all index tables
+ conn.createStatement().execute("alter table " + tableName + " set TTL=" + MODIFIED_TTL_VALUE
+ + ",KEEP_DELETED_CELLS=" + MODIFIED_KEEP_DELETED_CELLS_VALUE
+ + ",REPLICATION_SCOPE=" + MODIFIED_REPLICATION_SCOPE_VALUE);
+
+ for (String table: tablesToCheck) {
+ verifyHBaseColumnFamilyProperties(table, conn, true, false);
+ }
+
+ // Any indexes created henceforth should have the modified properties
+ String newGlobalIndex = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL);
+ String newViewIndex = createIndexTable(conn, tableName, null);
+ verifyHBaseColumnFamilyProperties(newGlobalIndex, conn, true, false);
+ verifyHBaseColumnFamilyProperties(newViewIndex, conn, true, false);
+ conn.close();
+ }
+
+ // Test that any if we add a column family to a base table, it gets the synced properties
+ @Test
+ public void testAlterTableAddColumnFamilyGetsSyncedProps() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+
+ // Test that we are not allowed to set any property to be kept in sync, specific to the new column family to be added
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute(
+ "alter table " + tableName + " add " + NEW_CF + ".new_column varchar(2) "
+ + NEW_CF + "." + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when altering synced property for a specific column family when adding a column");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced property for a specific column family when adding a column",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+
+ // Test that when we add a new column (belonging to a new column family) and set any property that should be
+ // in sync, then the property is modified for all existing column families of the base table and its indexes
+ Set<String> tablesToCheck = new HashSet<>();
+ tablesToCheck.add(tableName);
+ for (int i=0; i<2; i++) {
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.LOCAL));
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.GLOBAL));
+ }
+ // Create a view and view index
+ tablesToCheck.add(createIndexTable(conn, tableName, null));
+
+ // Now add a new column family while simultaneously modifying properties to be kept in sync,
+ // as well as a property which does not need to be kept in sync. Properties to be kept in sync
+ // should get propagated to all index tables and already existing column families
+ conn.createStatement().execute(
+ "alter table " + tableName + " add " + NEW_CF + ".new_column varchar(2) "
+ + "KEEP_DELETED_CELLS=" + MODIFIED_KEEP_DELETED_CELLS_VALUE
+ + ",REPLICATION_SCOPE=" + MODIFIED_REPLICATION_SCOPE_VALUE
+ + ",BLOCKSIZE=300000");
+
+ for (String table: tablesToCheck) {
+ verifyHBaseColumnFamilyProperties(table, conn, true, true);
+ }
+ try (Admin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin()) {
+ ColumnFamilyDescriptor[] columnFamilies = admin.getDescriptor(TableName.valueOf(tableName))
+ .getColumnFamilies();
+ for (ColumnFamilyDescriptor cfd: columnFamilies) {
+ if (cfd.getNameAsString().equals(NEW_CF)) {
+ assertEquals("Newly added column fmaily should have updated property",
+ 300000, cfd.getBlocksize());
+ } else {
+ assertEquals("Existing column families should have default value for property",
+ ColumnFamilyDescriptorBuilder.DEFAULT_BLOCKSIZE, cfd.getBlocksize());
+ }
+ }
+ }
+ conn.close();
+ }
+
+ // Test that we disallow altering a synced property for a global index table
+ @Test
+ public void testDisallowAlterGlobalIndexTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ String globalIndexName = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL);
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("alter table " + globalIndexName + " set "
+ + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when altering synced property for a global index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced property for a global index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+ conn.close();
+ }
+
+ // Test the upgrade code path for old client to new phoenix server cases in which the client
+ // may have tables which have column families and indexes whose properties are out of sync
+ @Test
+ public void testOldClientSyncPropsUpgradePath() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+
+ String baseTableName = createBaseTableWithProps(conn);
--- End diff --
It's not necessary. I just wanted to make sure the upgrade path works fine for multiple base tables
---
[GitHub] phoenix issue #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCO...
Posted by ChinmaySKulkarni <gi...@git.apache.org>.
Github user ChinmaySKulkarni commented on the issue:
https://github.com/apache/phoenix/pull/346
Hey @JamesRTaylor @twdsilva please review.
---
[GitHub] phoenix issue #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCO...
Posted by ChinmaySKulkarni <gi...@git.apache.org>.
Github user ChinmaySKulkarni commented on the issue:
https://github.com/apache/phoenix/pull/346
Thanks for the review @twdsilva. I have addressed the nits and squashed my commits.
---
[GitHub] phoenix issue #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCO...
Posted by twdsilva <gi...@git.apache.org>.
Github user twdsilva commented on the issue:
https://github.com/apache/phoenix/pull/346
+1 , Thanks @ChinmaySKulkarni I will get this committed.
---
[GitHub] phoenix pull request #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICAT...
Posted by ChinmaySKulkarni <gi...@git.apache.org>.
Github user ChinmaySKulkarni closed the pull request at:
https://github.com/apache/phoenix/pull/346
---
[GitHub] phoenix pull request #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICAT...
Posted by twdsilva <gi...@git.apache.org>.
Github user twdsilva commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/346#discussion_r219356971
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java ---
@@ -92,8 +95,22 @@
public static final byte[] VIEW_INDEX_SEQUENCE_PREFIX_BYTES = Bytes.toBytes(VIEW_INDEX_SEQUENCE_PREFIX);
private static final String VIEW_INDEX_ID_COLUMN_NAME = "_INDEX_ID";
public static final String PARENT_TABLE_KEY = "PARENT_TABLE";
- public static final byte[] PARENT_TABLE_KEY_BYTES = Bytes.toBytes("PARENT_TABLE");
-
+ public static final String IS_VIEW_INDEX_TABLE_PROP_NAME = "IS_VIEW_INDEX_TABLE";
+ public static final byte[] IS_VIEW_INDEX_TABLE_PROP_BYTES = Bytes.toBytes(IS_VIEW_INDEX_TABLE_PROP_NAME);
+
+ public static final String IS_LOCAL_INDEX_TABLE_PROP_NAME = "IS_LOCAL_INDEX_TABLE";
+ public static final byte[] IS_LOCAL_INDEX_TABLE_PROP_BYTES = Bytes.toBytes(IS_LOCAL_INDEX_TABLE_PROP_NAME);
+
+ public static final String DATA_TABLE_NAME_PROP_NAME = "DATA_TABLE_NAME";
+
+ public static final byte[] DATA_TABLE_NAME_PROP_BYTES = Bytes.toBytes(DATA_TABLE_NAME_PROP_NAME);
+
+ // See PHOENIX-3955
--- End diff --
nit: rename to SYNCED_DATA_TABLE_AND_INDEX_PROPERTIES
---
[GitHub] phoenix issue #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCO...
Posted by ChinmaySKulkarni <gi...@git.apache.org>.
Github user ChinmaySKulkarni commented on the issue:
https://github.com/apache/phoenix/pull/346
@twdsilva Ignoring non-Phoenix tables in the upgrade code path as we discussed today. I'll squash both commits and submit a patch once you're done reviewing. Thanks!
---
[GitHub] phoenix issue #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICATION_SCO...
Posted by twdsilva <gi...@git.apache.org>.
Github user twdsilva commented on the issue:
https://github.com/apache/phoenix/pull/346
+1
---
[GitHub] phoenix pull request #346: PHOENIX-3955: Ensure KEEP_DELETED_CELLS, REPLICAT...
Posted by twdsilva <gi...@git.apache.org>.
Github user twdsilva commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/346#discussion_r220293666
--- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/PropertiesInSyncIT.java ---
@@ -0,0 +1,462 @@
+/*
+ * 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.end2end;
+
+import org.apache.hadoop.hbase.KeepDeletedCells;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.schema.PTable;
+import org.apache.phoenix.util.MetaDataUtil;
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.MetaDataUtil.PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES;
+import static org.apache.phoenix.util.MetaDataUtil.VIEW_INDEX_TABLE_PREFIX;
+import static org.apache.phoenix.util.UpgradeUtil.syncTableAndIndexProperties;
+
+/**
+ * Test properties that need to be kept in sync amongst all column families and indexes of a table
+ */
+public class PropertiesInSyncIT extends ParallelStatsDisabledIT {
+ private static final String COL_FAM1 = "CF1";
+ private static final String COL_FAM2 = "CF2";
+ private static final String NEW_CF = "NEW_CF";
+ private static final String DUMMY_PROP_VALUE = "dummy";
+ private static final int INITIAL_TTL_VALUE = 700;
+ private static final KeepDeletedCells INITIAL_KEEP_DELETED_CELLS_VALUE = KeepDeletedCells.TRUE;
+ private static final int INITIAL_REPLICATION_SCOPE_VALUE = 1;
+ private static final int MODIFIED_TTL_VALUE = INITIAL_TTL_VALUE + 300;
+ private static final KeepDeletedCells MODIFIED_KEEP_DELETED_CELLS_VALUE =
+ (INITIAL_KEEP_DELETED_CELLS_VALUE == KeepDeletedCells.TRUE) ? KeepDeletedCells.FALSE: KeepDeletedCells.TRUE;
+ private static final int MODIFIED_REPLICATION_SCOPE_VALUE = (INITIAL_REPLICATION_SCOPE_VALUE == 1) ? 0 : 1;
+
+
+ // Test that we disallow specifying synced properties to be set per column family when creating a table
+ @Test
+ public void testDisallowSyncedPropsToBeSetColFamSpecificCreateTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = generateUniqueName();
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("create table " + tableName
+ + " (id INTEGER not null primary key, "
+ + COL_FAM1 + ".name varchar(10), " + COL_FAM2 + ".flag boolean) "
+ + COL_FAM1 + "." + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a specific column family");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a specific column family",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+ conn.close();
+ }
+
+ // Test that all column families have the same value of synced properties when creating a table
+ @Test
+ public void testSyncedPropsAllColFamsCreateTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ verifyHBaseColumnFamilyProperties(tableName, conn, false, false);
+ conn.close();
+ }
+
+ // Test that we disallow specifying synced properties to be set when creating an index on a physical table or a view
+ @Test
+ public void testDisallowSyncedPropsToBeSetCreateIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ String localIndexName = tableName + "_LOCAL_IDX";
+ String globalIndexName = tableName + "_GLOBAL_IDX";
+ String viewName = "VIEW_" + tableName;
+ conn.createStatement().execute("create view " + viewName
+ + " (new_col SMALLINT) as select * from " + tableName + " where id > 1");
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("create local index " + localIndexName
+ + " on " + tableName + "(name) "
+ + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a local index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a local index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ try {
+ conn.createStatement().execute("create index " + globalIndexName
+ + " on " + tableName + "(flag) "
+ + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a global index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a global index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ try {
+ conn.createStatement().execute("create index view_index"
+ + " on " + viewName + " (flag)" + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when setting synced property for a view index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to set synced property for a view index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+ conn.close();
+ }
+
+ // Test that indexes have the same value of synced properties as their base table
+ @Test
+ public void testSyncedPropsBaseTableCreateIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ createIndexTable(conn, tableName, PTable.IndexType.LOCAL);
+ String globalIndexName = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL);
+
+ // We pass the base table as the physical HBase table since our check includes checking the local index column family too
+ verifyHBaseColumnFamilyProperties(tableName, conn, false, false);
+ verifyHBaseColumnFamilyProperties(globalIndexName, conn, false, false);
+ conn.close();
+ }
+
+ // Test that the physical view index table has the same value of synced properties as its base table
+ @Test
+ public void testSyncedPropsBaseTableCreateViewIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ String viewIndexName = createIndexTable(conn, tableName, null);
+
+ verifyHBaseColumnFamilyProperties(tableName, conn, false, false);
+ verifyHBaseColumnFamilyProperties(viewIndexName, conn, false, false);
+ conn.close();
+ }
+
+ // Test that we disallow specifying synced properties to be set per column family when altering a table
+ @Test
+ public void testDisallowSyncedPropsToBeSetColFamSpecificAlterTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ StringBuilder alterAllSyncedPropsString = new StringBuilder();
+ String modPropString = COL_FAM1 + ".%s=" + DUMMY_PROP_VALUE + ",";
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("alter table " + tableName
+ + " set " + COL_FAM1 + "." + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when altering synced property for a specific column family");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced property for a specific column family",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ alterAllSyncedPropsString.append(String.format(modPropString, propName));
+ }
+
+ // Test the same when we try to set all of these properties at once
+ try {
+ conn.createStatement().execute("alter table " + tableName + " set "
+ + alterAllSyncedPropsString.substring(0, alterAllSyncedPropsString.length() - 1));
+ fail("Should fail with SQLException when altering synced properties for a specific column family");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced properties for a specific column family",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ conn.close();
+ }
+
+ // Test that any alteration of the synced properties gets propagated to all indexes and the physical view index table
+ @Test
+ public void testAlterSyncedPropsPropagateToAllIndexesAndViewIndex() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ Set<String> tablesToCheck = new HashSet<>();
+ tablesToCheck.add(tableName);
+ for (int i=0; i<2; i++) {
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.LOCAL));
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.GLOBAL));
+ }
+ // Create a view and view index
+ tablesToCheck.add(createIndexTable(conn, tableName, null));
+
+ // Now alter the base table's properties. This should get propagated to all index tables
+ conn.createStatement().execute("alter table " + tableName + " set TTL=" + MODIFIED_TTL_VALUE
+ + ",KEEP_DELETED_CELLS=" + MODIFIED_KEEP_DELETED_CELLS_VALUE
+ + ",REPLICATION_SCOPE=" + MODIFIED_REPLICATION_SCOPE_VALUE);
+
+ for (String table: tablesToCheck) {
+ verifyHBaseColumnFamilyProperties(table, conn, true, false);
+ }
+
+ // Any indexes created henceforth should have the modified properties
+ String newGlobalIndex = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL);
+ String newViewIndex = createIndexTable(conn, tableName, null);
+ verifyHBaseColumnFamilyProperties(newGlobalIndex, conn, true, false);
+ verifyHBaseColumnFamilyProperties(newViewIndex, conn, true, false);
+ conn.close();
+ }
+
+ // Test that any if we add a column family to a base table, it gets the synced properties
+ @Test
+ public void testAlterTableAddColumnFamilyGetsSyncedProps() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+
+ // Test that we are not allowed to set any property to be kept in sync, specific to the new column family to be added
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute(
+ "alter table " + tableName + " add " + NEW_CF + ".new_column varchar(2) "
+ + NEW_CF + "." + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when altering synced property for a specific column family when adding a column");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced property for a specific column family when adding a column",
+ SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+
+ // Test that when we add a new column (belonging to a new column family) and set any property that should be
+ // in sync, then the property is modified for all existing column families of the base table and its indexes
+ Set<String> tablesToCheck = new HashSet<>();
+ tablesToCheck.add(tableName);
+ for (int i=0; i<2; i++) {
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.LOCAL));
+ tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.GLOBAL));
+ }
+ // Create a view and view index
+ tablesToCheck.add(createIndexTable(conn, tableName, null));
+
+ // Now add a new column family while simultaneously modifying properties to be kept in sync,
+ // as well as a property which does not need to be kept in sync. Properties to be kept in sync
+ // should get propagated to all index tables and already existing column families
+ conn.createStatement().execute(
+ "alter table " + tableName + " add " + NEW_CF + ".new_column varchar(2) "
+ + "KEEP_DELETED_CELLS=" + MODIFIED_KEEP_DELETED_CELLS_VALUE
+ + ",REPLICATION_SCOPE=" + MODIFIED_REPLICATION_SCOPE_VALUE
+ + ",BLOCKSIZE=300000");
+
+ for (String table: tablesToCheck) {
+ verifyHBaseColumnFamilyProperties(table, conn, true, true);
+ }
+ try (Admin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin()) {
+ ColumnFamilyDescriptor[] columnFamilies = admin.getDescriptor(TableName.valueOf(tableName))
+ .getColumnFamilies();
+ for (ColumnFamilyDescriptor cfd: columnFamilies) {
+ if (cfd.getNameAsString().equals(NEW_CF)) {
+ assertEquals("Newly added column fmaily should have updated property",
+ 300000, cfd.getBlocksize());
+ } else {
+ assertEquals("Existing column families should have default value for property",
+ ColumnFamilyDescriptorBuilder.DEFAULT_BLOCKSIZE, cfd.getBlocksize());
+ }
+ }
+ }
+ conn.close();
+ }
+
+ // Test that we disallow altering a synced property for a global index table
+ @Test
+ public void testDisallowAlterGlobalIndexTable() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+ String tableName = createBaseTableWithProps(conn);
+ String globalIndexName = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL);
+ for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) {
+ try {
+ conn.createStatement().execute("alter table " + globalIndexName + " set "
+ + propName + "=" + DUMMY_PROP_VALUE);
+ fail("Should fail with SQLException when altering synced property for a global index");
+ } catch (SQLException sqlE) {
+ assertEquals("Should fail to alter synced property for a global index",
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode());
+ }
+ }
+ conn.close();
+ }
+
+ // Test the upgrade code path for old client to new phoenix server cases in which the client
+ // may have tables which have column families and indexes whose properties are out of sync
+ @Test
+ public void testOldClientSyncPropsUpgradePath() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl(), new Properties());
+
+ String baseTableName = createBaseTableWithProps(conn);
--- End diff --
Do you need to test for both baseTableName and baseTableName1?
---