You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2014/10/06 03:35:22 UTC

[4/5] git commit: PHOENIX-1325 Pass in instead of calculate if we've crossed a region boundary in ScanRanges intersect methods

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/c9101f82
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/c9101f82
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/c9101f82

Branch: refs/heads/4.0
Commit: c9101f827bf0ec0c76eedd6f91c746d9f4864506
Parents: 5df8d1e
Author: James Taylor <jt...@salesforce.com>
Authored: Sun Oct 5 10:48:11 2014 -0700
Committer: James Taylor <jt...@salesforce.com>
Committed: Sun Oct 5 10:48:11 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/c9101f82/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/c9101f82/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/c9101f82/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 66807b7..7c73918 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/c9101f82/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/c9101f82/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}")