> On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java > > Lines 52 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928238#file1928238line52> > > > > What does ``getId()`` return for a control node action?
the getId() for the ActionExecutors as I saw are following the pattern: JobId@ActionName. In case of the ControlNodeActionExecutors looks like: 0000000-171228181538280-oozie-mart-W@end, 0000000-171228181538280-oozie-mart-W@:start:, 0000004-171228181538280-oozie-mart-W@fs-paralel (this was my fork action), 0000004-171228181538280-oozie-mart-W@join-fs (this was my join action). But as we agreed I will remove it from the log message because is duplicated. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java > > Lines 59 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928238#file1928238line59> > > > > What does ``getExternalStatus()`` return for a control node action? It > > would might sense to log ``getStatus()`` too. In case of ControlNodeACtionExecutor the status is the same as the externalStatus (OK). It will be redundant to write both of them. There are cases when the two values are different, but the status is determined based on the value of the external status > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java > > Line 45 (original), 47 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928239#file1928239line47> > > > > What does ``getId()`` return for a control node action? I've just > > checked TestDecisionActionExecutor (executed) and saw it was ``null`` that > > does not seem very useful. > > > > Can you create a workflow with all changed control nodes and try it out > > on a cluster (pseudo hadoop should be fine) and double check logs about > > starting and ending actions? the getId() for the ActionExecutors as I saw are following the pattern: JobId@ActionName, where the action name comes from the workflow.xml, and here the name is mandatory. But as we discussed I will remove it from the log message because is duplicated. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java > > Lines 83 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928239#file1928239line87> > > > > There is a difference between status ``action.getStatus()`` (that can > > be "OK", "KILLED", "FAILED", etc.) and ``getExternalStatus()`` (in case of > > a decision action it returns where the execution will flow, transition to > > the next workflow action). > > > > For example: > > ``` > > <switch xmlns='uri:oozie:workflow:0.1'> > > <case to='a'>false</case> > > <case to='b'>false</case> > > <case to='c'>false</case> > > <default to='d'/> > > </switch > > ``` > > > > would return ``OK`` for ``action.getStatus()`` and ``d``for > > ``action.getExternalStatus()``. It might help to log both. Yes, you are right. The external status will return the transition. As we discussed it will be unnecessary to log both ot them. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java > > Lines 107 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928240#file1928240line107> > > > > What does ``getId()`` return? JobId@ActionName, where the action name comes from the workflow.xml. But as we discussed I will remove it from the log message because is duplicated. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java > > Lines 313 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928240#file1928240line313> > > > > What does ``getExternalStatus()`` return? It would might sense to log > > ``getStatus()`` too. In this case the two values are the same > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java > > Lines 630 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928241#file1928241line630> > > > > What does ``getId()`` return? JobId@ActionName, where the action name comes from the workflow.xml. But as we discussed I will remove it from the log message because is duplicated. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java > > Lines 167 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928242#file1928242line167> > > > > What does ``getId()`` return? JobId@ActionName, where the action name comes from the workflow.xml. But as we discussed I will remove it from the log message because is duplicated. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java > > Lines 259 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928242#file1928242line259> > > > > What does ``getExternalStatus()`` return? It would might sense to log > > ``getStatus()`` too. In case of the SubWorkflowActionExecutor the external status will be the same as the status. Here the external status is defined based on the status. So is unnecessary to log both of them. > On Dec. 28, 2017, 11:44 a.m., Attila Sasvari wrote: > > core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java > > Lines 467 (patched) > > <https://reviews.apache.org/r/64808/diff/2/?file=1928243#file1928243line470> > > > > What does ``getExternalStatus()`` return? It would might sense to log > > ``getStatus()`` too. Here the two values will be the same. Here the external status is defined based on the status. So is unnecessary to log both of them. - Kinga ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64808/#review194570 ----------------------------------------------------------- On Dec. 27, 2017, 9:12 a.m., Kinga Marton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64808/ > ----------------------------------------------------------- > > (Updated Dec. 27, 2017, 9:12 a.m.) > > > Review request for oozie and Attila Sasvari. > > > Repository: oozie-git > > > Description > ------- > > Decision, ForkJoin, Email and FS ActionExecutors are producing no or very > little logs on info level. This makes it hard to gather usage information and > help with troubleshooting in the case of any issues. Please improve logging > in these classes. > Several subtasks can be separated out from this one > > > Diffs > ----- > > > core/src/main/java/org/apache/oozie/action/control/ControlNodeActionExecutor.java > cc1b6108 > > core/src/main/java/org/apache/oozie/action/decision/DecisionActionExecutor.java > 8c235bda > core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java > d59f1d76 > core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java > 63f6104d > > core/src/main/java/org/apache/oozie/action/oozie/SubWorkflowActionExecutor.java > d62cf686 > core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java > 5890b8c1 > > > Diff: https://reviews.apache.org/r/64808/diff/2/ > > > Testing > ------- > > > Thanks, > > Kinga Marton > >
