Bug 51866 - Counter under loop doesn't work properly if "Start next loop on error" option set for thread group
Counter under loop doesn't work properly if "Start next loop on error" option...
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
2.5.1
All All
: P2 critical (vote)
: ---
Assigned To: JMeter issues mailing list
:
Depends on: 51868
Blocks:
  Show dependency tree
 
Reported: 2011-09-22 06:46 UTC by Oleg
Modified: 2012-01-15 13:46 UTC (History)
2 users (show)



Attachments
Scenario for reproducing error (12.35 KB, application/octet-stream)
2011-09-22 06:46 UTC, Oleg
Details
Fix to issue (4.57 KB, patch)
2011-10-02 06:51 UTC, Philippe Mouawad
Details | Diff
Simplified test case (19.33 KB, text/plain)
2011-10-04 22:51 UTC, Sebb
Details
Same patch as 51868 taking into account you last comment (6.12 KB, patch)
2011-10-05 13:33 UTC, Philippe Mouawad
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg 2011-09-22 06:46:14 UTC
Created attachment 27556 [details]
Scenario for reproducing error

My scenario has such structure:
TestPlan
+Thread (Start next loop on error)
++Loop (Loop Count = 7)
+++Counter (increment = 1)
+++Sampler1
+++Sampler2
If any sampler inside loop was failed, value for counter incremented by "Loop Count" for loop
Workflow:
Sampler1 (counter = 1)
Sampler2 (counter = 1)
Sampler1 (counter = 2)
Sampler2 (counter = 2)
Sampler1(failed)
Sampler1 (counter = 9)
Sampler2 (counter = 9)
It looks like after error all iterations of loop passed but samplers just disabled, counters - not
Comment 1 Philippe Mouawad 2011-10-01 19:56:14 UTC
Same explanation as 51868.
Incrementation is due to this code in JMeterThread:

sam = controller.next(); // need perfom a until loop for special case (tc as parent)
while (sam != null && !sam.equals(firstSampler)) { // while the thread is NOT on the begining of the tree
                                    sam = controller.next();
}
Comment 2 Philippe Mouawad 2011-10-02 06:51:55 UTC
Created attachment 27670 [details]
Fix to issue

Hello,
Patch uses same approach as 51868.
Regards
Philippe
Comment 3 Sebb 2011-10-04 22:51:58 UTC
Created attachment 27692 [details]
Simplified test case

It's not just the counter that behaves strangely.

The attached test case behaves differently if the "Outer" Java sampler is disabled. In that case, the counter increments correctly.
Comment 4 Sebb 2011-10-04 23:08:41 UTC
(In reply to comment #2)
> Created attachment 27670 [details]
> Fix to issue
> 
> Hello,
> Patch uses same approach as 51868.
> Regards
> Philippe

Although that might fix the Counter issue, it's not really scaleable - there are bound to be other elements that rely on the iterationStart() event: if not currently, then they may be added later.

The fix needs to ensure that the iterationStart() event is only triggered when the next loop actually starts.

Rather than looping through the samplers, maybe the code can trigger the loop-end condition in each of the parent controllers until the Thread Group is reached?

Unfortunately this area of JMeter code is very complicated and not all that well documented.
Comment 5 Philippe Mouawad 2011-10-05 13:33:30 UTC
Created attachment 27697 [details]
Same patch as 51868 taking into account you last comment

Regards
Philippe M.
Comment 6 Sebb 2011-10-05 15:45:30 UTC
Sorry, but I don't think that's OK either.

The method fireIterationStart() may be invoked by fireIterEvents() which resets first; it does not seem right to clear first when fireIterationStart() does nothing.

The code needs to behave as if each controller had reached the end of its loop, otherwise I think there are going to be edge cases that won't work, e.g. Once Only Controller.
Comment 7 Philippe Mouawad 2011-10-05 15:52:30 UTC
If I put JMeterContextService.getContext().isWithinRestartNextLoop() test 
in fireIterEvents() instead, 
do you see a case where it could fail ?

Regards
Philippe
Comment 8 Sebb 2011-10-05 16:06:50 UTC
(In reply to comment #7)
> If I put JMeterContextService.getContext().isWithinRestartNextLoop() test 
> in fireIterEvents() instead, 
> do you see a case where it could fail ?

Yes, if fireIterationStart() is called directly.
But adding it to both won't necessarily help either, as that only fixes the issue with iteration listeners.

But as I pointed out in Comment 3, it's not just the counter that misbehaves; the counter problem is just one symptom.

I think the whole "Start next loop" code needs rewriting.

Effectively the option means "go to end of loop" for each controller up to the Thread Group. [At least I assume this is the intention, as the option only appears on the Thread Group controller.]

So we need to code the feature as if this has happened, and then everything else should happen naturally.
Comment 9 Philippe Mouawad 2011-10-31 10:52:21 UTC
Date: Mon Oct 31 10:50:11 2011
New Revision: 1195404

URL: http://svn.apache.org/viewvc?rev=1195404&view=rev
Log:
Fix to Start Next Loop broken feature, fixes following issues:
- Bug 51865 - Infinite loop inside thread group does not work properly if "Start next loop after a Sample error" option set
- Bug 51868 - A lot of exceptions in jmeter.log while using option "Start next loop" for thread
- Bug 51866 - Counter under loop doesn't work properly if "Start next loop on error" option set for thread group


Added:
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/threads/FindTestElementsUpToRootTraverser.java   (with props)
Modified:
   jakarta/jmeter/trunk/src/components/org/apache/jmeter/control/ForeachController.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/Controller.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/GenericController.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/IfController.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/LoopController.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/RunTime.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/WhileController.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/threads/AbstractThreadGroup.java
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/threads/JMeterThread.java
   jakarta/jmeter/trunk/xdocs/changes.xml
Comment 10 Philippe Mouawad 2011-11-01 10:05:24 UTC
Hello,
A Fix has been provided, it would be Nice of you to test it and submit a comment confirming it is fixed for you.

Regards
Comment 11 Oleg 2011-12-13 13:41:06 UTC
looks like fixed, thanks
verified on r1213077 (nightly build)
Comment 12 Philippe Mouawad 2011-12-14 12:09:35 UTC
*** Bug 52330 has been marked as a duplicate of this bug. ***
Comment 13 Philippe Mouawad 2012-01-15 13:46:43 UTC
Thanks for verification.
Marking issue as RESOLVED to avoid it appearing in Bug Report.