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:
[email protected]
