Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/12#discussion_r17769890
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/DefaultParallelIteratorRegionSplitter.java
 ---
    @@ -117,121 +107,87 @@ public boolean apply(HRegionLocation location) {
         }
     
         protected List<KeyRange> genKeyRanges(List<HRegionLocation> regions) {
    -        if (regions.isEmpty()) {
    -            return Collections.emptyList();
    +        if (regions.isEmpty()) { return Collections.emptyList(); }
    +        Scan scan = context.getScan();
    +        PTable table = this.tableRef.getTable();
    +        byte[] defaultCF = SchemaUtil.getEmptyColumnFamily(table);
    +        List<byte[]> gps = null;
    +
    +        if (table.getColumnFamilies().isEmpty()) {
    +            // For sure we can get the defaultCF from the table
    +            gps = table.getGuidePosts();
    +        } else {
    +            try {
    +                if (scan.getFamilyMap().size() > 0) {
    +                    if (scan.getFamilyMap().containsKey(defaultCF)) { // 
Favor using default CF if it's used in scan
    +                        gps = 
table.getColumnFamily(defaultCF).getGuidePosts();
    +                    } else { // Otherwise, just use first CF in use by scan
    +                        gps = 
table.getColumnFamily(scan.getFamilyMap().keySet().iterator().next()).getGuidePosts();
    +                    }
    +                } else {
    +                    gps = table.getColumnFamily(defaultCF).getGuidePosts();
    +                }
    +            } catch (ColumnFamilyNotFoundException cfne) {
    +                // Alter table does this
    +            }
             }
             
    -        StatsManager statsManager = 
context.getConnection().getQueryServices().getStatsManager();
    -        // the splits are computed as follows:
    -        //
    -        // let's suppose:
    -        // t = target concurrency
    -        // m = max concurrency
    -        // r = the number of regions we need to scan
    -        //
    -        // if r >= t:
    -        //    scan using regional boundaries
    -        // elif r > t/2:
    -        //    split each region in s splits such that:
    -        //    s = max(x) where s * x < m
    -        // else:
    -        //    split each region in s splits such that:
    -        //    s = max(x) where s * x < t
    -        //
    -        // The idea is to align splits with region boundaries. If rows are 
not evenly
    -        // distributed across regions, using this scheme compensates for 
regions that
    -        // have more rows than others, by applying tighter splits and 
therefore spawning
    -        // off more scans over the overloaded regions.
    -        int splitsPerRegion = getSplitsPerRegion(regions.size());
    -        // Create a multi-map of ServerName to List<KeyRange> which we'll 
use to round robin from to ensure
    -        // that we keep each region server busy for each query.
    -        ListMultimap<HRegionLocation,KeyRange> keyRangesPerRegion = 
ArrayListMultimap.create(regions.size(),regions.size() * splitsPerRegion);;
    -        if (splitsPerRegion == 1) {
    +        List<KeyRange> guidePosts = 
Lists.newArrayListWithCapacity(regions.size());
    +        List<KeyRange> regionStartEndKey = 
Lists.newArrayListWithExpectedSize(regions.size());
    +        if (gps == null || gps.isEmpty()) {
    +            if (logger.isDebugEnabled()) {
    +                logger.debug("The splits formed from region start and 
endkeys are: " + regionStartEndKey);
    +            }
                 for (HRegionLocation region : regions) {
    -                keyRangesPerRegion.put(region, 
ParallelIterators.TO_KEY_RANGE.apply(region));
    +                
regionStartEndKey.add(KeyRange.getKeyRange(region.getRegionInfo().getStartKey(),
 region.getRegionInfo()
    +                        .getEndKey()));
                 }
    +            return regionStartEndKey;
             } else {
    --- End diff --
    
    Thanks for the review of my review, @ramkrish86. I believe this will work 
to replace the if/else. What do you think?
    
            byte[] currentKey = regions.get(0).getRegionInfo().getStartKey();
            byte[] endKey = null;
            int regionIndex = 0;
            int guideIndex = 0;
            int gpsSize = gps.size();
            int regionSize = regions.size();
            guideIndex = currentKey.length == 0 ? 0 : 
Collections.binarySearch(gps, currentKey, Bytes.BYTES_COMPARATOR);
            // Merge bisect with guideposts for all but the last region
            while (regionIndex < regionSize) {
                byte[] currentGuidePost;
                endKey = regions.get(regionIndex++).getRegionInfo().getEndKey();
                while (guideIndex < gpsSize && 
(Bytes.compareTo(currentGuidePost = gps.get(guideIndex), endKey) <= 0 || 
endKey.length == 0)) {
                    KeyRange keyRange = KeyRange.getKeyRange(currentKey, 
currentGuidePost);
                    if (keyRange != KeyRange.EMPTY_RANGE) {
                        guidePosts.add(keyRange);
                    }
                    currentKey = currentGuidePost;
                    guideIndex++;
                }
                KeyRange keyRange = KeyRange.getKeyRange(currentKey, endKey);
                if (keyRange != KeyRange.EMPTY_RANGE) {
                    guidePosts.add(keyRange);
                }
                currentKey = endKey;
            }
            if (logger.isDebugEnabled()) {
                logger.debug("The captured guideposts are: " + guidePosts);
            }
            return guidePosts;    



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

Reply via email to