Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/268#discussion_r129187099
--- 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);
+ /**
+ * 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) {
+ // Call once per server
+ if (remainingOnServers.contains(entry)
+ || (keys != null &&
ByteUtil.contains(keys,entry.getRegionInfo().getStartKey()))) {
--- End diff --
Why is this extra check required? Is this when the Hash Cache is sent over
to the RS that doesn't have it? If so, can we document that?
---
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.
---
