phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (PHOENIX-3600) Core MapReduce classes don't provide location info
Date Thu, 19 Jan 2017 21:15:26 GMT

    [ 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)

Mime
View raw message