shahrs87 commented on code in PR #1691:
URL: https://github.com/apache/phoenix/pull/1691#discussion_r1342003895


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3416,6 +3437,167 @@ private MetaDataMutationResult mutateColumn(
         }
     }
 
+    /**
+     * Invalidate metadata cache from all region servers for the given tenant 
and table name.
+     * @param tenantId
+     * @param schemaName
+     * @param tableOrViewName
+     * @throws Throwable
+     */
+    private void invalidateServerMetadataCache(byte[] tenantId, 
byte[]schemaName,
+            byte[] tableOrViewName) throws Throwable {
+        Configuration conf = env.getConfiguration();
+        String value = conf.get(REGIONSERVER_COPROCESSOR_CONF_KEY);
+        if (value == null
+                || 
!value.contains(PhoenixRegionServerEndpoint.class.getName())) {
+            // PhoenixRegionServerEndpoint is not loaded. We don't have to 
invalidate the cache.
+            LOGGER.info("Skip invalidating server metadata cache for tenantID: 
{},"
+                            + " schema name: {}, table Name: {} since 
PhoenixRegionServerEndpoint"
+                            + " is not loaded", Bytes.toString(tenantId),
+                    Bytes.toString(schemaName), 
Bytes.toString(tableOrViewName));
+            return;
+        }
+        Properties properties = new Properties();
+        // Skip checking of system table existence since the system tables 
should have created
+        // by now.
+        properties.setProperty(SKIP_SYSTEM_TABLES_EXISTENCE_CHECK, "true");
+        try (PhoenixConnection connection = 
QueryUtil.getConnectionOnServer(properties,
+                env.getConfiguration()).unwrap(PhoenixConnection.class);
+             Admin admin = connection.getQueryServices().getAdmin()) {
+            // This will incur an extra RPC to the master. This RPC is 
required since we want to
+            // get current list of regionservers.
+            Collection<ServerName> serverNames = admin.getRegionServers(true);
+            invalidateServerMetadataCacheWithRetries(admin, serverNames, 
tenantId, schemaName,
+                    tableOrViewName, false);
+        }
+    }
+
+    /**
+     * Invalidate metadata cache on all regionservers with retries for the 
given tenantID
+     * and tableName with retries. We retry once before failing the operation.
+     *
+     * @param admin
+     * @param serverNames
+     * @param tenantId
+     * @param schemaName
+     * @param tableOrViewName
+     * @param retryAttempt
+     * @throws Throwable
+     */
+    private void invalidateServerMetadataCacheWithRetries(Admin admin,
+            Collection<ServerName> serverNames, byte[] tenantId, byte[] 
schemaName,
+            byte[] tableOrViewName, boolean retryAttempt) throws Throwable {
+        String fullTableName = SchemaUtil.getTableName(schemaName, 
tableOrViewName);
+        String tenantIDStr = Bytes.toString(tenantId);
+        LOGGER.info("Invalidating metadata cache for tenantID: {}, tableName: 
{} for"
+                        + " region servers: {}, isRetryAttempt: {}", 
tenantIDStr, fullTableName,
+                serverNames, retryAttempt);
+        RegionServerEndpointProtos.InvalidateServerMetadataCacheRequest 
request =
+                getRequest(tenantId, schemaName, tableOrViewName);
+        // TODO Do I need my own executor or can I re-use 
QueryServices#Executor

Review Comment:
   >  Maybe it is better to create a new executor so that you can control its 
size
   
   @tkhurana  Do you think currently we can keep it as it is and use custom 
threadpool if needed? 



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to