You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/09/15 04:57:22 UTC
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #883: PHOENIX-6072: SYSTEM.MUTEX not created with a TTL on a fresh cluster connected to by a 4.15+ client
ChinmaySKulkarni commented on a change in pull request #883:
URL: https://github.com/apache/phoenix/pull/883#discussion_r488382762
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemTablesCreationOnConnectionIT.java
##########
@@ -18,6 +18,9 @@
package org.apache.phoenix.end2end;
Review comment:
A lot of these diffs are auto-formatting by my IDE to be according to Phoenix coding standards for whitespace, line length, etc.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java
##########
@@ -338,48 +448,53 @@
BIND_PARAMETERS + " VARCHAR, \n" +
SCAN_METRICS_JSON + " VARCHAR, \n" +
MetricType.getMetricColumnsDetails()+"\n"+
- " CONSTRAINT " + SYSTEM_TABLE_PK_NAME + " PRIMARY KEY (START_TIME, TABLE_NAME, QUERY_ID))\n" +
- PhoenixDatabaseMetaData.SALT_BUCKETS + "=%s,\n"+
- PhoenixDatabaseMetaData.TRANSACTIONAL + "=" + Boolean.FALSE+ ",\n" +
+ " CONSTRAINT " + SYSTEM_TABLE_PK_NAME +
+ " PRIMARY KEY (START_TIME, TABLE_NAME, QUERY_ID))\n" +
+ SALT_BUCKETS + "=%s,\n"+
+ TRANSACTIONAL + "=" + Boolean.FALSE+ ",\n" +
HColumnDescriptor.TTL + "=" + MetaDataProtocol.DEFAULT_LOG_TTL+",\n"+
- TableProperty.IMMUTABLE_STORAGE_SCHEME.toString() + " = " + ImmutableStorageScheme.SINGLE_CELL_ARRAY_WITH_OFFSETS.name() + ",\n" +
+ TableProperty.IMMUTABLE_STORAGE_SCHEME.toString() + " = " +
+ ImmutableStorageScheme.SINGLE_CELL_ARRAY_WITH_OFFSETS.name() + ",\n" +
TableProperty.COLUMN_ENCODED_BYTES.toString()+" = 1";
- public static final byte[] OFFSET_FAMILY = "f_offset".getBytes();
- public static final byte[] OFFSET_COLUMN = "c_offset".getBytes();
- public static final String LAST_SCAN = "LAST_SCAN";
- public static final byte[] UPGRADE_MUTEX = "UPGRADE_MUTEX".getBytes();
- public static final String HASH_JOIN_CACHE_RETRIES = "hashjoin.client.retries.number";
- public static final int DEFAULT_HASH_JOIN_CACHE_RETRIES = 5;
+ byte[] OFFSET_FAMILY = "f_offset".getBytes();
+ byte[] OFFSET_COLUMN = "c_offset".getBytes();
+ String LAST_SCAN = "LAST_SCAN";
+ String HASH_JOIN_CACHE_RETRIES = "hashjoin.client.retries.number";
+ int DEFAULT_HASH_JOIN_CACHE_RETRIES = 5;
// Links from parent to child views are stored in a separate table for
// scalability
- public static final String CREATE_CHILD_LINK_METADATA = "CREATE TABLE " + SYSTEM_CATALOG_SCHEMA + ".\"" +
+ String CREATE_CHILD_LINK_METADATA = "CREATE TABLE " + SYSTEM_CATALOG_SCHEMA + ".\"" +
SYSTEM_CHILD_LINK_TABLE + "\"(\n" +
// PK columns
- TENANT_ID + " VARCHAR NULL," + TABLE_SCHEM + " VARCHAR NULL," + TABLE_NAME + " VARCHAR NOT NULL,"
- + COLUMN_NAME + " VARCHAR NULL," + COLUMN_FAMILY + " VARCHAR NULL," + LINK_TYPE + " UNSIGNED_TINYINT,\n"
- + "CONSTRAINT " + SYSTEM_TABLE_PK_NAME + " PRIMARY KEY (" + TENANT_ID + "," + TABLE_SCHEM + "," + TABLE_NAME
+ TENANT_ID + " VARCHAR NULL," + TABLE_SCHEM + " VARCHAR NULL," + TABLE_NAME +
+ " VARCHAR NOT NULL," + COLUMN_NAME + " VARCHAR NULL," + COLUMN_FAMILY +
+ " VARCHAR NULL," + LINK_TYPE + " UNSIGNED_TINYINT,\n" + "CONSTRAINT " +
+ SYSTEM_TABLE_PK_NAME + " PRIMARY KEY (" +
+ TENANT_ID + "," + TABLE_SCHEM + "," + TABLE_NAME
+ "," + COLUMN_NAME + "," + COLUMN_FAMILY + "))\n" + HConstants.VERSIONS + "=%s,\n"
- + HColumnDescriptor.KEEP_DELETED_CELLS + "=%s,\n" + PhoenixDatabaseMetaData.TRANSACTIONAL + "="
+ + HColumnDescriptor.KEEP_DELETED_CELLS + "=%s,\n" + TRANSACTIONAL + "="
+ Boolean.FALSE;
-
- public static final String CREATE_MUTEX_METADTA =
- "CREATE IMMUTABLE TABLE " + SYSTEM_CATALOG_SCHEMA + ".\"" + SYSTEM_MUTEX_TABLE_NAME + "\"(\n" +
- // Pk columns
- TENANT_ID + " VARCHAR NULL," +
- TABLE_SCHEM + " VARCHAR NULL," +
- TABLE_NAME + " VARCHAR NOT NULL," +
- COLUMN_NAME + " VARCHAR NULL," + // null for table row
- COLUMN_FAMILY + " VARCHAR NULL " + // using for CF to uniqueness for columns
- "CONSTRAINT " + SYSTEM_TABLE_PK_NAME + " PRIMARY KEY (" + TENANT_ID + ","
- + TABLE_SCHEM + "," + TABLE_NAME + "," + COLUMN_NAME + "," + COLUMN_FAMILY + "))\n" +
- HConstants.VERSIONS + "=%s,\n" +
- HColumnDescriptor.KEEP_DELETED_CELLS + "=%s,\n" +
- PhoenixDatabaseMetaData.TRANSACTIONAL + "=" + Boolean.FALSE;
+
+ String CREATE_MUTEX_METADATA =
+ "CREATE IMMUTABLE TABLE " + SYSTEM_CATALOG_SCHEMA + ".\"" +
+ SYSTEM_MUTEX_TABLE_NAME + "\"(\n" +
+ // Pk columns
+ TENANT_ID + " VARCHAR NULL," +
+ TABLE_SCHEM + " VARCHAR NULL," +
+ TABLE_NAME + " VARCHAR NOT NULL," +
+ COLUMN_NAME + " VARCHAR NULL," + // null for table row
+ COLUMN_FAMILY + " VARCHAR NULL " + // using for CF to uniqueness for columns
+ "CONSTRAINT " + SYSTEM_TABLE_PK_NAME + " PRIMARY KEY (" + TENANT_ID + "," +
+ TABLE_SCHEM + "," + TABLE_NAME + "," + COLUMN_NAME + "," + COLUMN_FAMILY + "))\n" +
+ HConstants.VERSIONS + "=%s,\n" +
+ HColumnDescriptor.KEEP_DELETED_CELLS + "=%s,\n" +
+ TRANSACTIONAL + "=" + Boolean.FALSE + ",\n" +
+ HColumnDescriptor.TTL + "=" + TTL_FOR_MUTEX;
Review comment:
main change - part 2 (for fresh clusters starting with a 4.16 client)
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemTablesCreationOnConnectionIT.java
##########
@@ -18,6 +18,9 @@
package org.apache.phoenix.end2end;
Review comment:
Along with some changes to close statements, conns, etc. which were previously unclosed.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -3286,6 +3288,43 @@ void createSysMutexTableIfNotExists(HBaseAdmin admin) throws IOException, SQLExc
}
}
+ /**
+ * Check if the SYSTEM MUTEX table exists. If it does, ensure that its TTL is correct and if
+ * not, modify its table descriptor
+ * @param admin HBase admin
+ * @return true if SYSTEM MUTEX exists already and false if it needs to be created
+ * @throws IOException thrown if there is an error getting the table descriptor
+ */
+ @VisibleForTesting
+ boolean checkIfSysMutexExistsAndModifyTTLIfRequired(HBaseAdmin admin) throws IOException {
Review comment:
main change - part 1 (for the upgrade path used for clusters that have only seen 4.15 clients and will have a "FOREVER" TTL on SYSTEM.MUTEX)
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java
##########
@@ -40,6 +37,125 @@
import org.apache.phoenix.schema.SystemStatsSplitPolicy;
import org.apache.phoenix.schema.TableProperty;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.APPEND_ONLY_SCHEMA;
Review comment:
Most of these changes are also after passing through a linter which pointed out that `public static final` is unnecessary since this is an interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org