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_r361606430
 
 

 ##########
 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:
   Would you prefer this investigation and adding of tests be done under a 
separate JIRA? Or this one itself?

----------------------------------------------------------------
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]


With regards,
Apache Git Services

Reply via email to