Created attachment 36610 [details] Check for end of scheduler duration before the real sampling occurs Samplers might get started at the end of the scheduled duration, when they are configured with a delay. That delay has already been shortened to end at the end of duration with Bug 60049. That might lead to a stampede of samplers getting started at the end of a load test. When the tested system is too weak to handle this, those samples will take a lot of time. This will result in a strange looking report with high latency at the end of the test. To mitigate this, I propose to check for end of scheduler duration before right after the delaying logic inside the sampling setup.
@Felix, I think this fix should be put right into delay method, because it has extra delay that "prevents" delays that exceed scheduled timeframe. In other words, dealay method should stop scheduling rather than calling "adjustDelay".
Created attachment 36611 [details] Check for end of scheduler duration before the real sampling occurs End test in delay method, if the computed delay would result in waiting till the end of the scheduler duration.
Created attachment 36612 [details] Simple test plan to reproduce bug The attached test plan will not execute any samplers with this fix and end the scheduler directly. Without it, it will run for the scheduled period of time and execute the sampler afterwards.
Hi Felix, Can't we find a way to put this code in TimerService and add junit testing ? For example, make it return Pair<Boolean, Long> (exit or not/sleep time). As you noticed it, it's the 3rd bug around this feature. Thanks
(In reply to Philippe Mouawad from comment #4) > Hi Felix, > Can't we find a way to put this code in TimerService and add junit testing ? > > For example, make it return Pair<Boolean, Long> (exit or not/sleep time). I would probably go for an Optional<Long> and define an empty Optional to mean, we should end the test, but I am not sure, whether it is a good idea to create another object instance for this. Same could be expressed by returning a negative long. > > As you noticed it, it's the 3rd bug around this feature. The missing test case is one of the reasons, why I haven't committed it yet. > > Thanks
(In reply to Felix Schumacher from comment #5) > (In reply to Philippe Mouawad from comment #4) > > Hi Felix, > > Can't we find a way to put this code in TimerService and add junit testing ? > > > > For example, make it return Pair<Boolean, Long> (exit or not/sleep time). > > I would probably go for an Optional<Long> and define an empty Optional to > mean, we should end the test, but I am not sure, whether it is a good idea > to create another object instance for this. Same could be expressed by > returning a negative long. > +1 for this idea > > > > As you noticed it, it's the 3rd bug around this feature. > > The missing test case is one of the reasons, why I haven't committed it yet. Great, thanks > > > > > Thanks
> Optional<Long> and define an empty Optional to mean, we should end the test As for me, it would look cryptic.
(In reply to Vladimir Sitnikov from comment #7) > > Optional<Long> and define an empty Optional to mean, we should end the test > > As for me, it would look cryptic. Which one do you prefer? Or any other alternative?
Created attachment 36615 [details] Check for end of scheduler duration before the real sampling occurs The current logic of TimerService has now been changed to return -1, when delay would be too long for the end time. JMeterThread#delay stops the current thread, when delay is -1 (which could be surprising, when a timer uses -1 to indicate no waiting). A test case has been added to test the current behaviour (and an old one has been adapted to check for -1 in TimerService) and two tests have been added to complete all possible cases in adjustDelay.
(In reply to Felix Schumacher from comment #9) > Created attachment 36615 [details] > Check for end of scheduler duration before the real sampling occurs > > The current logic of TimerService has now been changed to return -1, when > delay would be too long for the end time. > > JMeterThread#delay stops the current thread, when delay is -1 (which could > be surprising, when a timer uses -1 to indicate no waiting). > > A test case has been added to test the current behaviour (and an old one has > been adapted to check for -1 in TimerService) and two tests have been added > to complete all possible cases in adjustDelay. +1 for merging
commit a72e68ca69e874be020b1bda5ccf3a5756e14f21 AuthorDate: Sun Jun 16 19:55:54 2019 +0200 At end of scheduler duration lots of Samplers gets executed at the same time The old behaviour lead to strange long sampling times at the end of scheduled test runs. Bugzilla Id: 63490 --- .../org/apache/jmeter/threads/JMeterThread.java | 14 +++ .../org/apache/jmeter/timers/TimerService.java | 4 +- .../apache/jmeter/threads/TestJMeterThread.java | 104 ++++++++++++++++++++- .../org/apache/jmeter/timers/TimerServiceTest.java | 17 +++- xdocs/changes.xml | 1 + 5 files changed, 135 insertions(+), 5 deletions(-) commit 2dc8f5b511dfb814c47402b348bc1aebe2991880 AuthorDate: Sun Jun 16 20:05:25 2019 +0200 Make checkstyle happy Use braces with if statements and use uppercase L for long literals. A bit of whitespace police while we are here anyway. Bugzilla Id: 63490
Closing after fix of Bug 63711 on nightly introduced by fix of this
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/5099