This is an automated email from the ASF dual-hosted git repository.
palashc 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 3ac1774213 PHOENIX-7381 : Skip last_ddl_timestamp validation when UCF
is defined on table and cache entry is not old (#1952)
3ac1774213 is described below
commit 3ac177421335a41d6e89b6cd36912c16f9f8983a
Author: Palash Chauhan <[email protected]>
AuthorDate: Mon Sep 30 23:35:13 2024 -0700
PHOENIX-7381 : Skip last_ddl_timestamp validation when UCF is defined on
table and cache entry is not old (#1952)
Co-authored-by: Palash Chauhan
<[email protected]>
---
.../org/apache/phoenix/schema/MetaDataClient.java | 21 +++++-----
.../java/org/apache/phoenix/util/MetaDataUtil.java | 8 ++++
.../phoenix/util/ValidateLastDDLTimestampUtil.java | 38 +++++++++++++++--
.../phoenix/cache/ServerMetadataCacheIT.java | 47 ++++++++++++++++++++++
.../UCFWithDisabledIndexWithDDLValidationIT.java | 14 +++----
5 files changed, 103 insertions(+), 25 deletions(-)
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
index 166caa6a66..407fce4db3 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
@@ -785,13 +785,13 @@ public class MetaDataClient {
final long effectiveUpdateCacheFreq;
final String ucfInfoForLogging; // Only used for logging purposes
- boolean overrideUcfToDefault = false;
+ boolean overrideUcfToAlways = false;
if (table.getType() == INDEX) {
- overrideUcfToDefault =
+ overrideUcfToAlways =
PIndexState.PENDING_DISABLE.equals(table.getIndexState()) ||
!IndexMaintainer.sendIndexMaintainer(table);
}
- if (!overrideUcfToDefault && !table.getIndexes().isEmpty()) {
+ if (!overrideUcfToAlways && !table.getIndexes().isEmpty()) {
List<PTable> indexes = table.getIndexes();
List<PTable> maintainedIndexes =
Lists.newArrayList(IndexMaintainer.maintainedIndexes(indexes.iterator()));
@@ -801,7 +801,7 @@ public class MetaDataClient {
// not in usable state by the client mutations, we should
override
// UPDATE_CACHE_FREQUENCY to default value so that we make
getTable() RPC calls
// until all index states change to ACTIVE, BUILDING or other
usable states.
- overrideUcfToDefault = indexes.size() !=
maintainedIndexes.size();
+ overrideUcfToAlways = indexes.size() !=
maintainedIndexes.size();
}
// What if the table is created with UPDATE_CACHE_FREQUENCY
explicitly set to ALWAYS?
@@ -810,12 +810,10 @@ public class MetaDataClient {
//always fetch an Index in PENDING_DISABLE state to retrieve
server timestamp
//QueryOptimizer needs that to decide whether the index can be used
- if (overrideUcfToDefault) {
+ if (overrideUcfToAlways) {
effectiveUpdateCacheFreq =
- (Long) ConnectionProperty.UPDATE_CACHE_FREQUENCY.getValue(
- connection.getQueryServices().getProps()
-
.get(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB));
- ucfInfoForLogging = "connection-level-default";
+ (Long)
ConnectionProperty.UPDATE_CACHE_FREQUENCY.getValue("ALWAYS");
+ ucfInfoForLogging = "override-to-always";
} else if (table.getUpdateCacheFrequency()
!= QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY) {
effectiveUpdateCacheFreq = table.getUpdateCacheFrequency();
@@ -836,9 +834,8 @@ public class MetaDataClient {
(table.getTenantId() != null ? ", Tenant ID: " +
table.getTenantId() : ""));
}
- return (table.getRowTimestampColPos() == -1 &&
- connection.getMetaDataCache().getAge(tableRef) <
- effectiveUpdateCacheFreq);
+ return MetaDataUtil
+ .avoidMetadataRPC(connection, table, tableRef,
effectiveUpdateCacheFreq);
}
return false;
}
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
index 2fa3ac109f..ca13846d9e 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
@@ -33,6 +33,7 @@ import org.apache.phoenix.schema.PColumnFamily;
import org.apache.phoenix.schema.PName;
import org.apache.phoenix.schema.PNameFactory;
import org.apache.phoenix.schema.PTable;
+import org.apache.phoenix.schema.PTableRef;
import org.apache.phoenix.schema.PTableType;
import org.apache.phoenix.schema.SequenceKey;
import org.apache.phoenix.schema.SortOrder;
@@ -1196,4 +1197,11 @@ public class MetaDataUtil {
return maxLookbackAge != null ? maxLookbackAge :
BaseScannerRegionObserverConstants.getMaxLookbackInMillis(conf);
}
+
+ public static boolean avoidMetadataRPC(PhoenixConnection connection,
PTable table,
+ PTableRef tableRef, long
effectiveUpdateCacheFreq) {
+ return table.getRowTimestampColPos() == -1 &&
+ connection.getMetaDataCache().getAge(tableRef) <
+ effectiveUpdateCacheFreq;
+ }
}
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java
index dbb321a244..95b1d1537a 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java
@@ -35,10 +35,13 @@ import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.query.QueryServicesOptions;
+import org.apache.phoenix.schema.ConnectionProperty;
import org.apache.phoenix.schema.PName;
import org.apache.phoenix.schema.PTable;
import org.apache.phoenix.schema.PTableKey;
+import org.apache.phoenix.schema.PTableRef;
import org.apache.phoenix.schema.PTableType;
+import org.apache.phoenix.schema.TableNotFoundException;
import org.apache.phoenix.schema.TableRef;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -100,7 +103,10 @@ public class ValidateLastDDLTimestampUtil {
public static void validateLastDDLTimestamp(PhoenixConnection conn,
List<TableRef> allTableRefs,
boolean doRetry) throws
SQLException {
- List<TableRef> tableRefs = filterTableRefs(allTableRefs);
+ List<TableRef> tableRefs = filterTableRefs(conn, allTableRefs);
+ if (tableRefs.isEmpty()) {
+ return;
+ }
String infoString = getInfoString(conn.getTenantId(), tableRefs);
try (Admin admin = conn.getQueryServices().getAdmin()) {
// get all live region servers
@@ -215,14 +221,38 @@ public class ValidateLastDDLTimestampUtil {
}
/**
- * Filter out any TableRefs which are not tables, views or indexes.
+ * Filter out TableRefs for sending to server to validate
last_ddl_timestamp.
+ * 1. table type is in ALLOWED_PTABLE_TYPES
+ * 2. table schema has a non-zero UPDATE_CACHE_FREQUENCY and cache entry
is old.
* @param tableRefs
* @return
*/
- private static List<TableRef> filterTableRefs(List<TableRef> tableRefs) {
+ private static List<TableRef> filterTableRefs(PhoenixConnection conn,
+ List<TableRef> tableRefs) {
List<TableRef> filteredTableRefs = tableRefs.stream()
- .filter(tableRef ->
ALLOWED_PTABLE_TYPES.contains(tableRef.getTable().getType()))
+ .filter(tableRef ->
ALLOWED_PTABLE_TYPES.contains(tableRef.getTable().getType())
+ && !avoidRpc(conn, tableRef.getTable()))
.collect(Collectors.toList());
return filteredTableRefs;
}
+
+ /**
+ * Decide whether we should avoid the validate timestamp RPC for this
table. If the schema of
+ * the table had specified a positive UCF to begin with, clients for this
table should not see
+ * a regression when metadata caching re-design is enabled i.e. any server
RPC should be
+ * skipped for them within the UCF window.
+ */
+ private static boolean avoidRpc(PhoenixConnection conn, PTable table) {
+ try {
+ PTableRef ptr = conn.getTableRef(table.getKey());
+ long tableUCF = table.getUpdateCacheFrequency();
+ return tableUCF > (Long)
ConnectionProperty.UPDATE_CACHE_FREQUENCY.getValue("ALWAYS")
+ && tableUCF < (Long)
ConnectionProperty.UPDATE_CACHE_FREQUENCY.getValue("NEVER")
+ && MetaDataUtil.avoidMetadataRPC(conn, table, ptr,
tableUCF);
+ } catch (TableNotFoundException e) {
+ //should not happen since this is called after query compilation
and optimizer
+ //so the table would be in the cache
+ return false;
+ }
+ }
}
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/cache/ServerMetadataCacheIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/cache/ServerMetadataCacheIT.java
index 1ecf1843c9..8e309cbc2f 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/cache/ServerMetadataCacheIT.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/cache/ServerMetadataCacheIT.java
@@ -1812,6 +1812,47 @@ public class ServerMetadataCacheIT extends
ParallelStatsDisabledIT {
}
}
+ /**
+ * Test that last_ddl_timestamp validation is skipped when client's
operation involves
+ * a table with 0<UCF<Long.MAX_VALUE.
+ */
+ @Test
+ public void testNoDDLTimestampValidationWithTableUCF() throws Exception {
+ String tableName = generateUniqueName();
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String url1 = QueryUtil.getConnectionUrl(props, config, "client1");
+ String url2 = QueryUtil.getConnectionUrl(props, config, "client2");
+ ConnectionQueryServices cqs1 = driver.getConnectionQueryServices(url1,
props);
+ ConnectionQueryServices cqs2 = driver.getConnectionQueryServices(url2,
props);
+ MetricsMetadataCachingSource metricsSource
+ =
MetricsPhoenixCoprocessorSourceFactory.getInstance().getMetadataCachingSource();
+ //take a snapshot of current metric values
+ MetricsMetadataCachingSource.MetadataCachingMetricValues
oldMetricValues
+ = metricsSource.getCurrentMetricValues();
+
+ try (Connection conn1 = cqs1.connect(url1, props);
+ Connection conn2 = cqs2.connect(url2, props)) {
+
+ // client-1 creates table with UCF=2days
+ createTableWithUCF(conn1, tableName, 2*24*60*60*1000L);
+
+ // query/upsert a few times with same and different client
+ for (int i=0; i<3; i++) {
+ query(conn2, tableName);
+ upsert(conn2, tableName, true);
+ query(conn1, tableName);
+ upsert(conn1, tableName, true);
+ }
+
+ MetricsMetadataCachingSource.MetadataCachingMetricValues
newMetricValues
+ = metricsSource.getCurrentMetricValues();
+
+ assertEquals("There should have been no timestamp validation
requests.",
+ 0,
+ newMetricValues.getValidateDDLTimestampRequestsCount()
+ -
oldMetricValues.getValidateDDLTimestampRequestsCount());
+ }
+ }
//Helper methods
public static void assertNumGetTableRPC(ConnectionQueryServices spyCqs,
String tableName, int numExpectedRPCs) throws SQLException {
@@ -1840,6 +1881,12 @@ public class ServerMetadataCacheIT extends
ParallelStatsDisabledIT {
+ "(k INTEGER NOT NULL PRIMARY KEY, v1 INTEGER, v2 INTEGER)");
}
+ private void createTableWithUCF(Connection conn, String tableName, long
ucf) throws SQLException {
+ conn.createStatement().execute("CREATE TABLE " + tableName
+ + "(k INTEGER NOT NULL PRIMARY KEY, v1 INTEGER, v2 INTEGER) "
+ + "UPDATE_CACHE_FREQUENCY=" + ucf);
+ }
+
private void createView(Connection conn, String parentName, String
viewName) throws SQLException {
conn.createStatement().execute("CREATE VIEW " + viewName + " AS SELECT
* FROM " + parentName);
}
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/UCFWithDisabledIndexWithDDLValidationIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/UCFWithDisabledIndexWithDDLValidationIT.java
index 1d0b87d52a..74235aaa57 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/UCFWithDisabledIndexWithDDLValidationIT.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/UCFWithDisabledIndexWithDDLValidationIT.java
@@ -40,7 +40,7 @@ public class UCFWithDisabledIndexWithDDLValidationIT extends
UCFWithDisabledInde
Map<String, String> props = Maps.newConcurrentMap();
props.put(BaseScannerRegionObserverConstants.PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY,
Integer.toString(60 * 60 * 1000));
- props.put(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB,
"ALWAYS");
+ props.put(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB,
"NEVER");
props.put(QueryServices.LAST_DDL_TIMESTAMP_VALIDATION_ENABLED,
Boolean.toString(true));
props.put(QueryServices.PHOENIX_METADATA_INVALIDATE_CACHE_ENABLED,
Boolean.toString(true));
props.put(QueryServices.TASK_HANDLING_INITIAL_DELAY_MS_ATTRIB,
@@ -55,26 +55,22 @@ public class UCFWithDisabledIndexWithDDLValidationIT
extends UCFWithDisabledInde
@Test
public void testUcfWithNoGetTableCalls() throws Throwable {
- // Uncomment with PHOENIX-7381
- //super.testUcfWithNoGetTableCalls();
+ super.testUcfWithNoGetTableCalls();
}
@Test
public void testUcfWithDisabledIndex1() throws Throwable {
- // Uncomment with PHOENIX-7381
- //super.testUcfWithDisabledIndex1();
+ super.testUcfWithDisabledIndex1();
}
@Test
public void testUcfWithDisabledIndex2() throws Throwable {
- // Uncomment with PHOENIX-7381
- //super.testUcfWithDisabledIndex2();
+ super.testUcfWithDisabledIndex2();
}
@Test
public void testUcfWithDisabledIndex3() throws Throwable {
- // Uncomment with PHOENIX-7381
- //super.testUcfWithDisabledIndex3();
+ super.testUcfWithDisabledIndex3();
}
}
