[
https://issues.apache.org/jira/browse/PHOENIX-3600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830617#comment-15830617
]
Josh Elser commented on PHOENIX-3600:
-------------------------------------
+1 looking at the code. Some minor things that could be cleaned up, but I trust
you can do those on commit.
Repeating myself from PHOENIX-3601: I don't have a setup to test this easily,
but I'd like to get one in place. Assuming I get to it, I'll let you know how
some testing works out.
{code}
+ psplits.add(new
PhoenixInputSplit(Lists.<Scan>newArrayList(aScan), regionSize, regionLocation));
{code}
Nit: could use {{Collections.singletonList()}} instead since we know it's only
ever going to be one element (I think Guava over-estimates a little bit).
{code}
/**
* No Arg constructor
*/
public PhoenixInputSplit() {
}
-
/**
{code}
Nit: Unnecessary whitespace change.
{code}
@@ -58,6 +58,15 @@ public class PhoenixInputSplit extends InputSplit implements
Writable {
this.scans = scans;
init();
}
{code}
Do a {{this(scans, 0, null)}} instead?
{code}
@Override
public String[] getLocations() throws IOException, InterruptedException {
- return new String[]{};
+ return new String[]{regionLocation};
}
{code}
Should this only happen when {{regionLocation}} is non-null? It's not obvious
just looking at the javadoc for {{InputSplit}}.
{code}
+ // Generate splits based on scans from stats, or just from region splits
+ public static final String MAPREDUCE_SPLIT_BY_STATS =
"phoenix.mapreduce.split.by.stats";
+
+ public static final boolean DEFAULT_SPLIT_BY_STATS = true;
{code}
Would be good to make sure this change in default action propagates up to the
docs.
> Core MapReduce classes don't provide location info
> --------------------------------------------------
>
> Key: PHOENIX-3600
> URL: https://issues.apache.org/jira/browse/PHOENIX-3600
> Project: Phoenix
> Issue Type: Improvement
> Affects Versions: 4.8.0
> Reporter: Josh Mahonin
> Assignee: Josh Mahonin
> Attachments: PHOENIX-3600.patch
>
>
> The core MapReduce classes {{org.apache.phoenix.mapreduce.PhoenixInputSplit}}
> and {{org.apache.phoenix.mapreduce.PhoenixInputFormat}} don't provide region
> size or location information, leaving the execution engine (MR, Spark, etc.)
> to randomly assign splits to nodes.
> Interestingly, the phoenix-hive module has reimplemented these classes,
> including the node-aware functionality. We should port a subset of those
> changes back to the core code so that other engines can make use of them.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
