[jira] [Comment Edited] (OOZIE-2827) More directly view of the coordinator’s history from perspective of workflow action.

["Peter Bacsko (JIRA)" <ji...@apache.org>]

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

Peter Bacsko edited comment on OOZIE-2827 at 3/31/17 10:23 AM:
---------------------------------------------------------------

I have three comments:

* Please add a short {{// no-op}} comment to the empty method bodies (to indicate that the implementation is missing on purpose)
* Here, we just swallow the exception and set {{wfAction}} to null - I don't have full context, but it looks like code smell:

{code}
                    try {
                        wfAction = jpaService.execute(new WorkflowActionGetJPAExecutor(wfActionId));
                    } catch (JPAExecutorException ex) {
                        wfAction = null;
                    }
{code}

* Another thing right here that looks suspicious is that even if {{wfAction}} is null, we still add it to the list:
{code}
                        wfAction = null;
                    }
                } else {
                    wfAction = null;
                }
                wfActions.add(wfAction);
{code}

Again, I'm not fully 100% familiar with this code, but if {{wfId}} is null or we get an exception, we should treat is as an error and throw an exception (I'm not sure if {{wfId}} being null is really an error or not, but it feels like an erroneous behaviour).


was (Author: pbacsko):
I have two comments:

* Please add a short {{// no-op}} comment to the empty method bodies (to indicate that the implementation is missing on purpose)
* Here, we just swallow the exception and set {{wfAction}} to null - I don't have full context, but it looks like code smell:

{code}
                    try {
                        wfAction = jpaService.execute(new WorkflowActionGetJPAExecutor(wfActionId));
                    } catch (JPAExecutorException ex) {
                        wfAction = null;
                    }
{code}

* Another thing right here that looks suspicious is that even if {{wfAction}} is null, we still add it to the list:
{code}
                        wfAction = null;
                    }
                } else {
                    wfAction = null;
                }
                wfActions.add(wfAction);
{code}

Again, I'm not fully 100% familiar with this code, but if {{wfId}} is null or we get an exception, we should treat is as an error and throw an exception (I'm not sure if {{wfId}} being null is really an error or not, but it feels like an erroneous behaviour).

> More directly view of the coordinator’s history from perspective of workflow action.
> ------------------------------------------------------------------------------------
>
>                 Key: OOZIE-2827
>                 URL: https://issues.apache.org/jira/browse/OOZIE-2827
>             Project: Oozie
>          Issue Type: Improvement
>            Reporter: Alonzo Zhou
>         Attachments: OOZIE-2827-0001.patch, OOZIE-2827-0002.patch
>
>
> Background: 
> When we monitor a coordinator job, statistics of historical workflow instances such as running time can be directly visualized.
> But, we lack the ability to monitor historical statistics of coordinator from perspective of  workflow action.For example, running time of a coordinator action is longer than before, but we can't see the exact change tendency of a sepcific workflow action in it。
> Functional optimization: 
> More detailed view of the coordinator’s history can be observed from perspective of  workflow action.Furthermore,  the trend of a workflow action can be shown directly on front-end graph.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)