ntshmah commented on a change in pull request #659: PHOENIX-5634: Use
'phoenix.default.update.cache.frequency' from conne…
URL: https://github.com/apache/phoenix/pull/659#discussion_r366512414
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
##########
@@ -722,14 +722,27 @@ public MetaDataMutationResult updateCache(PName
origTenantId, String schemaName,
// 1. table is a system table that does not have a ROW_TIMESTAMP column OR
// 2. table was already resolved as of that timestamp OR
// 3. table does not have a ROW_TIMESTAMP column and age is less then
UPDATE_CACHE_FREQUENCY
+ // 3a. Get the effective UPDATE_CACHE_FREQUENCY for checking the age in
the following precedence order:
+ // Table-level property > Connection-level property > Default value.
private boolean avoidRpcToGetTable(boolean alwaysHitServer, Long
resolvedTimestamp,
boolean systemTable, PTable table, PTableRef tableRef, long
tableResolvedTimestamp) {
- return table != null && !alwaysHitServer &&
- (systemTable && table.getRowTimestampColPos() == -1 ||
- resolvedTimestamp == tableResolvedTimestamp ||
- (table.getRowTimestampColPos() == -1 &&
- connection.getMetaDataCache().getAge(tableRef)
<
- table.getUpdateCacheFrequency()));
+ if (table != null && !alwaysHitServer) {
+ if (systemTable && table.getRowTimestampColPos() == -1 ||
resolvedTimestamp == tableResolvedTimestamp) {
+ return true;
+ }
+
+ // What if the table is created with UPDATE_CACHE_FREQUENCY
explicitly set to ALWAYS (= 0)?
+ // We should ideally be checking something like
hasUpdateCacheFrequency()
+ long effectiveUpdateCacheFrequency =
table.getUpdateCacheFrequency() != 0L ?
+ table.getUpdateCacheFrequency() :
+ connection.getQueryServices().getProps().getLong(
+
QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB,
QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY);
+
+ return (table.getRowTimestampColPos() == -1 &&
+ connection.getMetaDataCache().getAge(tableRef) <
+ effectiveUpdateCacheFrequency);
+ }
+ return false;
Review comment:
I've modified the tests to also check for the UPSERT path. UPSERT follows
the same behavior as SELECT.
> I guess for DDL operations, we force the client to get the latest metadata
That's not true. We have logic in PhoenixStatement#executeMutation() to
[force update cache if we run into meta not found
exceptions](https://github.com/apache/phoenix/blob/master/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java#L427-L437).
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
