Github user karanmehta93 commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/397#discussion_r229061395
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/iterate/MapReduceParallelScanGrouper.java
---
@@ -80,18 +83,39 @@ public boolean shouldStartNewScan(QueryPlan plan,
List<Scan> scans,
}
}
+ /**
+ * Get list of region locations from SnapshotManifest
+ * BaseResultIterators assume that regions are sorted using
RegionInfo.COMPARATOR
+ */
private List<HRegionLocation>
getRegionLocationsFromManifest(SnapshotManifest manifest) {
List<SnapshotRegionManifest> regionManifests =
manifest.getRegionManifests();
Preconditions.checkNotNull(regionManifests);
- List<HRegionLocation> regionLocations =
Lists.newArrayListWithCapacity(regionManifests.size());
+ List<RegionInfo> regionInfos =
Lists.newArrayListWithCapacity(regionManifests.size());
+ List<HRegionLocation> hRegionLocations =
Lists.newArrayListWithCapacity(regionManifests.size());
for (SnapshotRegionManifest regionManifest : regionManifests) {
- regionLocations.add(new HRegionLocation(
-
ProtobufUtil.toRegionInfo(regionManifest.getRegionInfo()), null));
+ RegionInfo regionInfo =
ProtobufUtil.toRegionInfo(regionManifest.getRegionInfo());
+ if (isValidRegion(regionInfo)) {
+ regionInfos.add(regionInfo);
+ }
+ }
+
+ regionInfos.sort(RegionInfo.COMPARATOR);
+
+ for (RegionInfo regionInfo : regionInfos) {
+ hRegionLocations.add(new HRegionLocation(regionInfo,
null));
}
- return regionLocations;
+ return hRegionLocations;
+ }
+
+ // Exclude offline split parent regions
+ private boolean isValidRegion(RegionInfo hri) {
--- End diff --
I thought about it, however they don't have any classes in common and the
implementations of these methods are slightly different. I agree that this is
duplicate code and should not exist this way but I couldn't think of a better
way for this.
---
