Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/268#discussion_r129186906
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java ---
@@ -303,73 +318,84 @@ public TaskExecutionMetricsHolder
getTaskExecutionMetric() {
* @throws SQLException
* @throws IllegalStateException if hashed table cannot be removed on
any region server on which it was added
*/
- private void removeServerCache(final byte[] cacheId,
Set<HRegionLocation> servers) throws SQLException {
- ConnectionQueryServices services =
connection.getQueryServices();
- Throwable lastThrowable = null;
- TableRef cacheUsingTableRef =
cacheUsingTableRefMap.get(Bytes.mapKey(cacheId));
- final PTable cacheUsingTable = cacheUsingTableRef.getTable();
- byte[] tableName =
cacheUsingTableRef.getTable().getPhysicalName().getBytes();
- HTableInterface iterateOverTable = services.getTable(tableName);
- try {
- List<HRegionLocation> locations =
services.getAllTableRegions(tableName);
- Set<HRegionLocation> remainingOnServers = new
HashSet<HRegionLocation>(servers);
- /**
- * Allow for the possibility that the region we based
where to send our cache has split and been
- * relocated to another region server *after* we sent
it, but before we removed it. To accommodate
- * this, we iterate through the current metadata
boundaries and remove the cache once for each
- * server that we originally sent to.
- */
- if (LOG.isDebugEnabled())
{LOG.debug(addCustomAnnotations("Removing Cache " + cacheId + " from servers.",
connection));}
- for (HRegionLocation entry : locations) {
- if (remainingOnServers.contains(entry)) { //
Call once per server
- try {
+ private void removeServerCache(final ServerCache cache,
Set<HRegionLocation> servers) throws SQLException {
+ HTableInterface iterateOverTable = null;
+ final byte[] cacheId = cache.getId();
+ try {
+ List<byte[]> keys = cache.getKeys();
+ ConnectionQueryServices services =
connection.getQueryServices();
+ Throwable lastThrowable = null;
+ TableRef cacheUsingTableRef =
cacheUsingTableRefMap.get(Bytes.mapKey(cacheId));
+ final PTable cacheUsingTable = cacheUsingTableRef.getTable();
+ byte[] tableName =
cacheUsingTableRef.getTable().getPhysicalName().getBytes();
+ iterateOverTable = services.getTable(tableName);
+
+ List<HRegionLocation> locations =
services.getAllTableRegions(tableName);
+ Set<HRegionLocation> remainingOnServers = new
HashSet<HRegionLocation>(servers);
--- End diff --
Instead of Set<HRegionLocation> can we change this to Set<ServerName>? It'd
make it more clear IMHO
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
