PHOENIX-1325 Pass in instead of calculate if we've crossed a region boundary in ScanRanges intersect methods
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/846a8b52 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/846a8b52 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/846a8b52 Branch: refs/heads/master Commit: 846a8b52eaf12245352cb64da72e1d8c17f4a8c9 Parents: 56a0c85 Author: James Taylor <jtay...@salesforce.com> Authored: Sun Oct 5 10:48:11 2014 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Sun Oct 5 18:47:26 2014 -0700 ---------------------------------------------------------------------- .../apache/phoenix/cache/ServerCacheClient.java | 3 ++- .../org/apache/phoenix/compile/ScanRanges.java | 28 +++++++++++++------- .../phoenix/iterate/ParallelIterators.java | 4 +-- .../compile/ScanRangesIntersectTest.java | 2 +- .../apache/phoenix/compile/ScanRangesTest.java | 2 +- 5 files changed, 24 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/846a8b52/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java b/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java index f22f874..ba7d265 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java @@ -172,7 +172,8 @@ public class ServerCacheClient { if ( ! servers.contains(entry) && keyRanges.intersects(regionStartKey, regionEndKey, cacheUsingTable.getIndexType() == IndexType.LOCAL ? - ScanUtil.getRowKeyOffset(regionStartKey, regionEndKey) : 0)) { // Call RPC once per server + ScanUtil.getRowKeyOffset(regionStartKey, regionEndKey) : 0, true)) { + // Call RPC once per server servers.add(entry); if (LOG.isDebugEnabled()) {LOG.debug(addCustomAnnotations("Adding cache entry to be sent for " + entry, connection));} final byte[] key = entry.getRegionInfo().getStartKey(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/846a8b52/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java index 4591bdb..923bcf3 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java @@ -207,7 +207,7 @@ public class ScanRanges { return temp; } - public Scan intersectScan(Scan scan, final byte[] originalStartKey, final byte[] originalStopKey, final int keyOffset) { + public Scan intersectScan(Scan scan, final byte[] originalStartKey, final byte[] originalStopKey, final int keyOffset, boolean crossesRegionBoundary) { byte[] startKey = originalStartKey; byte[] stopKey = originalStopKey; if (stopKey.length > 0 && Bytes.compareTo(startKey, stopKey) >= 0) { @@ -218,16 +218,22 @@ public class ScanRanges { // salt bytes in that case. final int scanKeyOffset = this.isSalted && !this.isPointLookup ? SaltingUtil.NUM_SALTING_BYTES : 0; assert (scanKeyOffset == 0 || keyOffset == 0); - // Offset for startKey/stopKey. Either 1 for salted tables or the prefix length - // of the current region for local indexes. + // Total offset for startKey/stopKey. Either 1 for salted tables or the prefix length + // of the current region for local indexes. We'll never have a case where a table is + // both salted and local. final int totalKeyOffset = scanKeyOffset + keyOffset; - // In this case, we've crossed the "prefix" boundary and should consider everything after the startKey - // This prevents us from having to prefix the key prior to knowing whether or not there may be an - // intersection. byte[] prefixBytes = ByteUtil.EMPTY_BYTE_ARRAY; if (totalKeyOffset > 0) { prefixBytes = ScanUtil.getPrefix(startKey, totalKeyOffset); - if (ScanUtil.crossesPrefixBoundary(stopKey, prefixBytes, totalKeyOffset)) { + /* + * If our startKey to stopKey crosses a region boundary consider everything after the startKey as our scan + * is always done within a single region. This prevents us from having to prefix the key prior to knowing + * whether or not there may be an intersection. We can't calculate whether or not we've crossed a region + * boundary for local indexes, because we don't know the key offset of the next region, but only for the + * current one (which is the one passed in). If the next prefix happened to be a subset of the previous + * prefix, then this wouldn't detect that we crossed a region boundary. + */ + if (crossesRegionBoundary) { stopKey = ByteUtil.EMPTY_BYTE_ARRAY; } } @@ -352,17 +358,19 @@ public class ScanRanges { /** * Return true if the range formed by the lowerInclusiveKey and upperExclusiveKey - * intersects with any of the scan ranges and false otherwise. We cannot pass in + * intersects with the scan ranges and false otherwise. We cannot pass in * a KeyRange here, because the underlying compare functions expect lower inclusive * and upper exclusive keys. We cannot get their next key because the key must * conform to the row key schema and if a null byte is added to a lower inclusive * key, it's no longer a valid, real key. * @param lowerInclusiveKey lower inclusive key * @param upperExclusiveKey upper exclusive key + * @param crossesRegionBoundary whether or not the upperExclusiveKey spans upto + * or after the next region. * @return true if the scan range intersects with the specified lower/upper key * range */ - public boolean intersects(byte[] lowerInclusiveKey, byte[] upperExclusiveKey, int keyOffset) { + public boolean intersects(byte[] lowerInclusiveKey, byte[] upperExclusiveKey, int keyOffset, boolean crossesRegionBoundary) { if (isEverything()) { return true; } @@ -371,7 +379,7 @@ public class ScanRanges { } //return filter.hasIntersect(lowerInclusiveKey, upperExclusiveKey); - return intersectScan(null, lowerInclusiveKey, upperExclusiveKey, keyOffset) == HAS_INTERSECTION; + return intersectScan(null, lowerInclusiveKey, upperExclusiveKey, keyOffset, crossesRegionBoundary) == HAS_INTERSECTION; } public SkipScanFilter getSkipScanFilter() { http://git-wip-us.apache.org/repos/asf/phoenix/blob/846a8b52/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java b/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java index f9e2d4b..b5a3d16 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java @@ -426,12 +426,12 @@ public class ParallelIterators extends ExplainTable implements ResultIterators { } while (guideIndex < gpsSize && (Bytes.compareTo(currentGuidePost = gps.get(guideIndex), endKey) <= 0 || endKey.length == 0)) { - Scan newScan = scanRanges.intersectScan(scan, currentKey, currentGuidePost, keyOffset); + Scan newScan = scanRanges.intersectScan(scan, currentKey, currentGuidePost, keyOffset, false); scans = addNewScan(parallelScans, scans, newScan, false); currentKey = currentGuidePost; guideIndex++; } - Scan newScan = scanRanges.intersectScan(scan, currentKey, endKey, keyOffset); + Scan newScan = scanRanges.intersectScan(scan, currentKey, endKey, keyOffset, true); scans = addNewScan(parallelScans, scans, newScan, true); currentKey = endKey; regionIndex++; http://git-wip-us.apache.org/repos/asf/phoenix/blob/846a8b52/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesIntersectTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesIntersectTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesIntersectTest.java index be90399..6f3dccc 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesIntersectTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesIntersectTest.java @@ -58,7 +58,7 @@ public class ScanRangesIntersectTest { scan.setFilter(ranges.getSkipScanFilter()); byte[] startKey = lowerRange == null ? KeyRange.UNBOUND : PDataType.VARCHAR.toBytes(lowerRange); byte[] stopKey = upperRange == null ? KeyRange.UNBOUND : PDataType.VARCHAR.toBytes(upperRange); - Scan newScan = ranges.intersectScan(scan, startKey, stopKey, 0); + Scan newScan = ranges.intersectScan(scan, startKey, stopKey, 0, true); if (expectedPoints.length == 0) { assertNull(newScan); } else { http://git-wip-us.apache.org/repos/asf/phoenix/blob/846a8b52/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesTest.java index 695c4c9..279d866 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/ScanRangesTest.java @@ -72,7 +72,7 @@ public class ScanRangesTest { // incrementing the key too much. upperExclusiveKey = ByteUtil.nextKey(upperExclusiveKey); } - assertEquals(expectedResult, scanRanges.intersects(lowerInclusiveKey,upperExclusiveKey,0)); + assertEquals(expectedResult, scanRanges.intersects(lowerInclusiveKey,upperExclusiveKey,0, true)); } @Parameters(name="{0} {2}")