From 4f8b056771a5935f6d2aacfe1fe3f22deb963ef3 Mon Sep 17 00:00:00 2001 From: Palash Chauhan Date: Mon, 12 Aug 2024 19:45:10 -0700 Subject: [PATCH 1/4] skip ddl validation when UCF is defined on table and cache entry is not old --- .../util/ValidateLastDDLTimestampUtil.java | 35 ++++++++++++-- .../phoenix/cache/ServerMetadataCacheIT.java | 47 +++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) 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 dbb321a2443..7f4f88651ed 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.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 static boolean getValidateLastDdlTimestampEnabled(Configuration config) { public static void validateLastDDLTimestamp(PhoenixConnection conn, List allTableRefs, boolean doRetry) throws SQLException { - List tableRefs = filterTableRefs(allTableRefs); + List 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,35 @@ private static void setLastDDLTimestampRequestParameters( } /** - * 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 filterTableRefs(List tableRefs) { + private static List filterTableRefs(PhoenixConnection conn, + List tableRefs) { List 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 > 0 && tableUCF < Long.MAX_VALUE + && conn.getMetaDataCache().getAge(ptr) < table.getUpdateCacheFrequency(); + } catch (TableNotFoundException e) { + return true; + } + } } 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 1ecf1843c98..8e309cbc2ff 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 void testCacheUpdatedBeforeDDLOperations() throws Exception { } } + /** + * Test that last_ddl_timestamp validation is skipped when client's operation involves + * a table with 0 Date: Tue, 13 Aug 2024 09:26:58 -0700 Subject: [PATCH 2/4] case when table is not found in cache --- .../org/apache/phoenix/util/ValidateLastDDLTimestampUtil.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 7f4f88651ed..264e3c52224 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 @@ -249,7 +249,9 @@ private static boolean avoidRpc(PhoenixConnection conn, PTable table) { return tableUCF > 0 && tableUCF < Long.MAX_VALUE && conn.getMetaDataCache().getAge(ptr) < table.getUpdateCacheFrequency(); } catch (TableNotFoundException e) { - return true; + //should not happen since this is called after query compilation and optimizer + //so the table would be in the cache + return false; } } } From 8bc878d407f82c34fbe4bb52134affb63f30e3c8 Mon Sep 17 00:00:00 2001 From: Palash Chauhan Date: Mon, 30 Sep 2024 16:19:02 -0700 Subject: [PATCH 3/4] small refactor and fix tests --- .../apache/phoenix/schema/MetaDataClient.java | 21 ++++++++----------- .../org/apache/phoenix/util/MetaDataUtil.java | 8 +++++++ .../util/ValidateLastDDLTimestampUtil.java | 5 +++-- ...FWithDisabledIndexWithDDLValidationIT.java | 14 +++++-------- 4 files changed, 25 insertions(+), 23 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 166caa6a660..8f308388efe 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 @@ private boolean avoidRpcToGetTable(boolean alwaysHitServer, Long resolvedTimesta 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 indexes = table.getIndexes(); List maintainedIndexes = Lists.newArrayList(IndexMaintainer.maintainedIndexes(indexes.iterator())); @@ -801,7 +801,7 @@ private boolean avoidRpcToGetTable(boolean alwaysHitServer, Long resolvedTimesta // 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 @@ private boolean avoidRpcToGetTable(boolean alwaysHitServer, Long resolvedTimesta //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 = "index-level-override"; } else if (table.getUpdateCacheFrequency() != QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY) { effectiveUpdateCacheFreq = table.getUpdateCacheFrequency(); @@ -836,9 +834,8 @@ private boolean avoidRpcToGetTable(boolean alwaysHitServer, Long resolvedTimesta (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 2fa3ac109fd..ca13846d9e3 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.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 static long getMaxLookbackAge(Configuration conf, Long maxLookbackAge) { 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 264e3c52224..95b1d1537a3 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 @@ -246,8 +246,9 @@ private static boolean avoidRpc(PhoenixConnection conn, PTable table) { try { PTableRef ptr = conn.getTableRef(table.getKey()); long tableUCF = table.getUpdateCacheFrequency(); - return tableUCF > 0 && tableUCF < Long.MAX_VALUE - && conn.getMetaDataCache().getAge(ptr) < 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 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 1d0b87d52a1..74235aaa577 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 @@ private static void initCluster() throws Exception { Map 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 static synchronized void doSetup() throws Exception { @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(); } } From 2af5beedc5de2dcce5c9c80301ff9a8634e2cc04 Mon Sep 17 00:00:00 2001 From: Palash Chauhan Date: Mon, 30 Sep 2024 18:46:49 -0700 Subject: [PATCH 4/4] change log info when overriding ucf --- .../src/main/java/org/apache/phoenix/schema/MetaDataClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8f308388efe..407fce4db3e 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 @@ -813,7 +813,7 @@ private boolean avoidRpcToGetTable(boolean alwaysHitServer, Long resolvedTimesta if (overrideUcfToAlways) { effectiveUpdateCacheFreq = (Long) ConnectionProperty.UPDATE_CACHE_FREQUENCY.getValue("ALWAYS"); - ucfInfoForLogging = "index-level-override"; + ucfInfoForLogging = "override-to-always"; } else if (table.getUpdateCacheFrequency() != QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY) { effectiveUpdateCacheFreq = table.getUpdateCacheFrequency();