Преглед на файлове

HPCC-9306 Don't allow restarted workunits to take the wrong branch

This fix ensures that conditions for conditional workflow actions are always
re-evaluated.  Otherwise, the condition can evaluate to true, the true branch
fails.  The workunit is resubmitted using "recover", the condition wasn't
re-evaluated, causing the false branch to be taken the second time.

The same issue could occur if there was a failure in a persist, possibly leading
to an internal error, or the persist not being re-created correctly.

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday преди 12 години
родител
ревизия
d6e6d9671c
променени са 4 файла, в които са добавени 47 реда и са изтрити 9 реда
  1. 10 7
      common/workunit/workflow.cpp
  2. 1 1
      common/workunit/workflow.hpp
  3. 1 1
      ecl/eclagent/eclagent.cpp
  4. 35 0
      testing/ecl/failrestart.ecl

+ 10 - 7
common/workunit/workflow.cpp

@@ -748,23 +748,26 @@ bool WorkflowMachine::doExecuteItemDependencies(IRuntimeWorkflowItem & item, uns
     Owned<IWorkflowDependencyIterator> iter = item.getDependencies();
     for(iter->first(); iter->isValid(); iter->next())
     {
-        if (!doExecuteItemDependency(item, iter->query(), scheduledWfid))
+        if (!doExecuteItemDependency(item, iter->query(), scheduledWfid, false))
             return false;
     }
     return true;
 }
 
-bool WorkflowMachine::doExecuteItemDependency(IRuntimeWorkflowItem & item, unsigned dep, unsigned scheduledWfid)
+bool WorkflowMachine::doExecuteItemDependency(IRuntimeWorkflowItem & item, unsigned wfid, unsigned scheduledWfid, bool alwaysEvaluate)
 {
     try
     {
-        return executeItem(dep, scheduledWfid);
+        if (alwaysEvaluate)
+            workflow->queryWfid(wfid).setState(WFStateNull);
+
+        return executeItem(wfid, scheduledWfid);
     }
     catch(WorkflowException * e)
     {
         if(e->queryType() == WorkflowException::ABORT)
             throw;
-        if(!attemptRetry(item, dep, scheduledWfid))
+        if(!attemptRetry(item, wfid, scheduledWfid))
         {
             handleFailure(item, e, true);
             throw;
@@ -819,12 +822,12 @@ bool WorkflowMachine::doExecuteConditionItem(IRuntimeWorkflowItem & item, unsign
     if(iter->next()) wfidFalse = iter->query();
     if(iter->next()) throwUnexpected();
 
-    if (!doExecuteItemDependency(item, wfidCondition, scheduledWfid))
+    if (!doExecuteItemDependency(item, wfidCondition, scheduledWfid, true))
         return false;
     if(condition)
-        return doExecuteItemDependency(item, wfidTrue, scheduledWfid);
+        return doExecuteItemDependency(item, wfidTrue, scheduledWfid, false);
     else if (wfidFalse)
-        return doExecuteItemDependency(item, wfidFalse, scheduledWfid);
+        return doExecuteItemDependency(item, wfidFalse, scheduledWfid, false);
     return true;
 }
 

+ 1 - 1
common/workunit/workflow.hpp

@@ -96,7 +96,7 @@ protected:
 
     // Iterate through dependencies and execute them
     bool doExecuteItemDependencies(IRuntimeWorkflowItem & item, unsigned scheduledWfid);
-    bool doExecuteItemDependency(IRuntimeWorkflowItem & item, unsigned dep, unsigned scheduledWfid);
+    bool doExecuteItemDependency(IRuntimeWorkflowItem & item, unsigned dep, unsigned scheduledWfid, bool alwaysEvaluate);
     // Execute an item (wrapper to deal with exceptions)
     void doExecuteItem(IRuntimeWorkflowItem & item, unsigned scheduledWfid);
     // Actually executes item: calls process->perform()

+ 1 - 1
ecl/eclagent/eclagent.cpp

@@ -2218,7 +2218,7 @@ void EclAgentWorkflowMachine::doExecutePersistItem(IRuntimeWorkflowItem & item)
         agent.decachePersist(name.str());
     else
         agent.startPersist(name.str());
-    doExecuteItemDependency(item, item.queryPersistWfid(), wfid);
+    doExecuteItemDependency(item, item.queryPersistWfid(), wfid, true);
     if(!persist)
     {
         StringBuffer errmsg;

+ 35 - 0
testing/ecl/failrestart.ecl

@@ -0,0 +1,35 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2012 HPCC Systems.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+############################################################################## */
+
+//The first time this workunit executes it gets an exception in a conditional workflow item.
+//If it is "recovered" the condition must be re-evaluated (otherwise the false branch is taken the
+//second time.
+
+o0 := OUTPUT('...');
+o1 := FAIL('Oh no!') : independent;
+o2 := OUTPUT('Should not be output') : independent;
+o3 := output('Begin');
+o4 := output('Done');
+
+b1 := SEQUENTIAL(o0, o1);
+b2 := SEQUENTIAL(o0, o2);
+
+cond := true : STORED('cond');
+
+x := IF(cond, b1, b2);
+
+SEQUENTIAL(o3, x, o4);