[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15093505#comment-15093505 ] Siddharth Seth commented on TEZ-2669: - bq. The choice of not having Exception in the API's but then wrapping all plugins inside wrapper classes to add an exception to the effective internal signature seems incongruous. Simply having the Exception in the signature makes things clear and informs implementations that its ok to throw an exception and notify the framework vs forcing them to swallow any-and-all exceptions because the method signature does not allow throwing exceptions. It is different from signatures on other interfaces. However, I believe this model works better since users have to think about how to handle Exceptions - either catch and do something useful, or catch and throw a Runtime. The framework cannot do the first part about doing something useful. As an example - something like invokeRpc() throws IOException. This, with Exception on the plugin signature, could end up being invoked once and the exception trickles down to the framework. Without an exception on the plugin signature, the user needs to decide on what to do - which could be a retry. I've also heard from users that a 'generic throws Exception()' is not very useful. bq. Could you please remind me why TaskCommunicator.java is inside tez-dag instead of inside tez-api/serviceplugins like the rest of the plugins? And how does Hive/LLAP end up implementing it in that state? Primarily because of it's dependencies on IDs. Also things like TezEvent. All of this needs to change at some point in the future. LLAP depends on tez-dag to implement it's own TaskCommunicator. [~zjffdu], [~bikassaha], [~hitesh] - any other comments? > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15094443#comment-15094443 ] Bikas Saha commented on TEZ-2669: - Implementations should think about which exception matter to them and which dont. Trying to motivate them to think about exceptions by not having it in the API signature seems overdone (given the additional unnecessary code being added to actually have exceptions after all internally). It would be nice to hear from other reviewers what they think about this choice. If others are ok with it then we could commit this. Else it probably needs some review. So I defer to the comments of other reviewers. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15094573#comment-15094573 ] Hitesh Shah commented on TEZ-2669: -- [~bikassaha] any comments? > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15094568#comment-15094568 ] Siddharth Seth commented on TEZ-2669: - bq. Thinking about this a bit more, can we consider declaring a checked exception along the lines of say "throws ServiceFatalException" ? This could be derived from either generic Exception or TezException. This would force plugin impls to only throw this declared exception in case of fatal errors. The exception handling code does not need to change i.e. should still catch Exception to account for unchecked exceptions thrown from plugins. Makes sense. Think this works better actually - since users are still forced to handle internal exceptions, and have an indication of what to throw. Will make the relevant changes. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15094968#comment-15094968 ] Bikas Saha commented on TEZ-2669: - ServicePluginException sounds good. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15094121#comment-15094121 ] Hitesh Shah commented on TEZ-2669: -- Unused import related to org.apache.avro.reflect.Nullable. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15095188#comment-15095188 ] Siddharth Seth commented on TEZ-2669: - Thanks everyone for the reviews. Committing. Test failures are unrelated. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt, > TEZ-2669.4.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[
https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15095179#comment-15095179
]
TezQA commented on TEZ-2669:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12781929/TEZ-2669.4.txt
against master revision 85637c6.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 21 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 3.0.1) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in :
org.apache.tez.test.TestFaultTolerance
Test results:
https://builds.apache.org/job/PreCommit-TEZ-Build/1420//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1420//console
This message is automatically generated.
> Propagation of errors from plugins to the AM for error reporting
>
>
> Key: TEZ-2669
> URL: https://issues.apache.org/jira/browse/TEZ-2669
> Project: Apache Tez
> Issue Type: Sub-task
>Affects Versions: 0.8.0-alpha
>Reporter: Siddharth Seth
>Assignee: Siddharth Seth
>Priority: Blocker
> Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt,
> TEZ-2669.4.txt
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15095184#comment-15095184 ] Hitesh Shah commented on TEZ-2669: -- +1 > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt, > TEZ-2669.4.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15093369#comment-15093369 ] Bikas Saha commented on TEZ-2669: - The choice of not having Exception in the API's but then wrapping all plugins inside wrapper classes to add an exception to the effective internal signature seems incongruous. Simply having the Exception in the signature makes things clear and informs implementations that its ok to throw an exception and notify the framework vs forcing them to swallow any-and-all exceptions because the method signature does not allow throwing exceptions. However, if other reviewers are ok with this choice then I am fine with it. Could you please remind me why TaskCommunicator.java is inside tez-dag instead of inside tez-api/serviceplugins like the rest of the plugins? And how does Hive/LLAP end up implementing it in that state? > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[
https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15089966#comment-15089966
]
TezQA commented on TEZ-2669:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12781121/TEZ-2669.3.txt
against master revision d5c9649.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 20 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 3.0.1) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in :
org.apache.tez.test.TestFaultTolerance
Test results:
https://builds.apache.org/job/PreCommit-TEZ-Build/1411//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1411//console
This message is automatically generated.
> Propagation of errors from plugins to the AM for error reporting
>
>
> Key: TEZ-2669
> URL: https://issues.apache.org/jira/browse/TEZ-2669
> Project: Apache Tez
> Issue Type: Sub-task
>Affects Versions: 0.8.0-alpha
>Reporter: Siddharth Seth
>Assignee: Siddharth Seth
>Priority: Blocker
> Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15088348#comment-15088348 ] Siddharth Seth commented on TEZ-2669: - Thanks for taking a look [~zjffdu]. Have gotten rid of the duplication. I don't want to get rid of the INTERNAL_ERROR sent out by the TaskScheduler in this patch - it's already very big. Have created TEZ-3028 to track this. Likewise - there's an onError API on the TaskSchedulerContext which doesn't exist on the other service plugins - that needs to be a separate jira. TEZ-3029 for this. bq. What's the purpose of TaskCommunicatorWrapper/ContainerLauncherWrapper/TaskSchedulerWrapper ? Add exception on methods ? We can do it on TaskCommunicator/ContainerLauncher/TaskScheduler directly since the plugin api is not stabilized yet I guess. Or maybe you don't want to make impact on downstream project (hive) ? >From my previous comment - The itnerfaces itself do not throw Exceptions - I >don't think it's useful doing that since Tez can't take any useful action in >such cases. This also forces the user code to look at possible Exceptions >instead of just letting them filter through to the framework. The Wrappers are a mechanism to force the framework to catch the Exceptions and handle them correctly. There's tests to ensure the Wrappers throw Exceptions for the relevant methods. This patch actually breaks the interface by removing exceptions from onVertexStateUpdated. The change is not to maintain compatibility - I don't think it's useful having an Exception on the methods. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15088509#comment-15088509 ] Bikas Saha commented on TEZ-2669: - Sorry for delayed response. Will look at this in a bit. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: 0.8.0-alpha >Reporter: Siddharth Seth >Assignee: Siddharth Seth >Priority: Blocker > Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[
https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15088684#comment-15088684
]
TezQA commented on TEZ-2669:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12781121/TEZ-2669.3.txt
against master revision d5c9649.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 20 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 3.0.1) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in :
org.apache.tez.test.TestFaultTolerance
Test results:
https://builds.apache.org/job/PreCommit-TEZ-Build/1408//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1408//console
This message is automatically generated.
> Propagation of errors from plugins to the AM for error reporting
>
>
> Key: TEZ-2669
> URL: https://issues.apache.org/jira/browse/TEZ-2669
> Project: Apache Tez
> Issue Type: Sub-task
>Affects Versions: 0.8.0-alpha
>Reporter: Siddharth Seth
>Assignee: Siddharth Seth
>Priority: Blocker
> Attachments: TEZ-2669.1.txt, TEZ-2669.2.txt, TEZ-2669.3.txt
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[
https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15086833#comment-15086833
]
TezQA commented on TEZ-2669:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12780892/TEZ-2669.1.txt
against master revision 9816a49.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 20 new
or modified test files.
{color:red}-1 javac{color}. The applied patch generated 40 javac
compiler warnings (more than the master's current 17 warnings).
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:red}-1 findbugs{color}. The patch appears to introduce 1 new
Findbugs (version 3.0.1) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-TEZ-Build/1404//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-TEZ-Build/1404//artifact/patchprocess/newPatchFindbugsWarningstez-dag.html
Javac warnings:
https://builds.apache.org/job/PreCommit-TEZ-Build/1404//artifact/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1404//console
This message is automatically generated.
> Propagation of errors from plugins to the AM for error reporting
>
>
> Key: TEZ-2669
> URL: https://issues.apache.org/jira/browse/TEZ-2669
> Project: Apache Tez
> Issue Type: Sub-task
>Affects Versions: 0.8.0-alpha
>Reporter: Siddharth Seth
>Assignee: Siddharth Seth
>Priority: Blocker
> Attachments: TEZ-2669.1.txt
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2669) Propagation of errors from plugins to the AM for error reporting
[ https://issues.apache.org/jira/browse/TEZ-2669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15062720#comment-15062720 ] Bikas Saha commented on TEZ-2669: - Changing priority based on resolved duplicate priority. > Propagation of errors from plugins to the AM for error reporting > > > Key: TEZ-2669 > URL: https://issues.apache.org/jira/browse/TEZ-2669 > Project: Apache Tez > Issue Type: Sub-task >Affects Versions: TEZ-2003 >Reporter: Siddharth Seth >Priority: Blocker > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
