[
https://issues.apache.org/jira/browse/PIG-161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12591608#action_12591608
]
Shravan Matthur Narayanamurthy commented on PIG-161:
----------------------------------------------------
I haven't gotten fully through this, but I have a couple of comments:
1) We need to be using the combiner aggressively out of the gate. I expect
MRCompiler is the place you'll want to use it. It should be handling placing
things in the compile stage. If you want to do it in a second pass over the
MROpPlan after MRCompiler is done, I think that's fine. But we need to have
this as part of our initial work. I'd like to resolve whether we're going to do
the combiner in MRCompiler or a separate pass before I commit this code.
wasn't sure if they
[shrav] - I was thinking that these and other optimizations would go into a
separate optimization layer which would operatre on the MROpPlan. That way, we
do not cloud the translation logic with optimizations. I have also put in some
basic infrastructure like the combine plan in the MROper and methods giving
access to the map and reduce plan as well. Also there are methods to access the
input plan and the compiled plan in the MRCompiler. I am sure there might be
others needed for the optimizer to function efficiently. But I feel that there
is enough flexibility to incorporate them.
2) In several places in MRCompiler you have unexpected if/else chains ending in
'log.warn("didn't expect this")'. These should be runtime exceptions I suspect.
[shrav] Yeah, you are right. I will change them.
3) Rather than the MergeException you created, let's create a general
PlanException. I can then change OperatorPlan to throw that instead of an
IOException, and you can use it in your merge stuff. I'll make that change.
[shrav] Sure
[ Show ยป ]
Alan Gates - 21/Apr/08 01:52 PM I haven't gotten fully through this, but I have
a couple of comments: 1) We need to be using the combiner aggressively out of
the gate. I expect MRCompiler is the place you'll want to use it. It should be
handling placing things in the compile stage. If you want to do it in a second
pass over the MROpPlan after MRCompiler is done, I think that's fine. But we
need to have this as part of our initial work. I'd like to resolve whether
we're going to do the combiner in MRCompiler or a separate pass before I commit
this code. 2) In several places in MRCompiler you have unexpected if/else
chains ending in 'log.warn("didn't expect this")'. These should be runtime
exceptions I suspect. 3) Rather than the MergeException you created, let's
create a general PlanException. I can then change OperatorPlan to throw that
instead of an IOException, and you can use it in your merge stuff. I'll make
that change.
> Rework physical plan
> --------------------
>
> Key: PIG-161
> URL: https://issues.apache.org/jira/browse/PIG-161
> Project: Pig
> Issue Type: Sub-task
> Reporter: Alan Gates
> Assignee: Alan Gates
> Attachments: arithmeticOperators.patch, incr2.patch, incr3.patch,
> incr4.patch, incr5.patch, MRCompilerTests_PlansAndOutputs.txt,
> Phy_AbsClass.patch, physicalOps.patch, podistinct.patch, pogenerate.patch,
> pogenerate.patch, pogenerate.patch, posort.patch
>
>
> This bug tracks work to rework all of the physical operators as described in
> http://wiki.apache.org/pig/PigTypesFunctionalSpec
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
