[ 
https://issues.apache.org/jira/browse/PIG-3419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13744218#comment-13744218
 ] 

Cheolsoo Park commented on PIG-3419:
------------------------------------

[~achalsoni81], thank you very much for the great work! Although I am not a Pig 
old-timer, I have been playing with your patch and have a few high-level 
comments as follows:
# I love your ExecType interface. What I don't like is that there are two 
things called ExecType:
{code}
org.apache.pig.ExecType
org.apache.pig.backend.executionengine.ExecType
{code}
Since you're introducing the ServiceLoader framework, the enum seems no longer 
needed at all. Furthermore, eliminating it helps simplify the constructor code 
of PigContext and PigServer.
# Currently, it is a bit hard to review your patch because there are too many 
changes. To get it committed faster, I suggest we should avoid unnecessary 
changes and minimize its scope. For example, having the PigServer constructor 
signature unchanged helps avoid a lot of changes in Test*.java files.
# Julien already pointed out this in the RB, but your patch accidentally 
reverts a couple of previous commits. I took them out.

I went ahead and made these changes myself - 
[here|https://github.com/piaozhexiu/apache-pig/commit/9aee192116816ecf993d69a8c2f94abaff7f9739].
 If everyone thinks this is a step forward, I will upload it in a new patch. 
Please let me know.
                
> Pluggable Execution Engine 
> ---------------------------
>
>                 Key: PIG-3419
>                 URL: https://issues.apache.org/jira/browse/PIG-3419
>             Project: Pig
>          Issue Type: New Feature
>    Affects Versions: 0.12
>            Reporter: Achal Soni
>            Assignee: Achal Soni
>            Priority: Minor
>         Attachments: execengine.patch, mapreduce_execengine.patch, 
> stats_scriptstate.patch, test_suite.patch
>
>
> In an effort to adapt Pig to work using Apache Tez 
> (https://issues.apache.org/jira/browse/TEZ), I made some changes to allow for 
> a cleaner ExecutionEngine abstraction than existed before. The changes are 
> not that major as Pig was already relatively abstracted out between the 
> frontend and backend. The changes in the attached commit are essentially the 
> barebones changes -- I tried to not change the structure of Pig's different 
> components too much. I think it will be interesting to see in the future how 
> we can refactor more areas of Pig to really honor this abstraction between 
> the frontend and backend. 
> Some of the changes was to reinstate an ExecutionEngine interface to tie 
> together the front end and backend, and making the changes in Pig to delegate 
> to the EE when necessary, and creating an MRExecutionEngine that implements 
> this interface. Other work included changing ExecType to cycle through the 
> ExecutionEngines on the classpath and select the appropriate one (this is 
> done using Java ServiceLoader, exactly how MapReduce does for choosing the 
> framework to use between local and distributed mode). Also I tried to make 
> ScriptState, JobStats, and PigStats as abstract as possible in its current 
> state. I think in the future some work will need to be done here to perhaps 
> re-evaluate the usage of ScriptState and the responsibilities of the 
> different statistics classes. I haven't touched the PPNL, but I think more 
> abstraction is needed here, perhaps in a separate patch. 

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to