JMeter shutdown using timers releases thundering herd of interrupted samplers

[asfimport]

Antony Bowesman (Bug 61380):
Seems like there's a flaw in the shutdown logic

When a test is running, it goes through JMeterThread. executeSamplePackage() where it goes into the delay(pack.getTimers()) call.

If you shut down a test, it will interrupt the timers and you will see these messages in jmeter.log

jmeter.threads.JMeterThread: The delay timer was interrupted - probably did not wait as long as intended

As a result, all threads blocked waiting to sample() will get released at pretty much the same time and as the JMeter iteration logic does the delay before the sample() rather than after, it means all threads will go into sample() at about the same time.

We've had cases where we've had to stop a production test as we're breaking it and the shutdown exacerbates the problem.

As the JMeterThread.stop() sets the running flag to false, it's probably a simple fix to wrap the code following the delay() call with if (running) {

}

Severity: normal
OS: All

[asfimport]

Sebb (migrated from Bugzilla):
(In reply to Antony Bowesman from comment 0)

Seems like there's a flaw in the shutdown logic

When a test is running, it goes through JMeterThread. executeSamplePackage()
where it goes into the delay(pack.getTimers()) call.

If you shut down a test, it will interrupt the timers and you will see these
messages in jmeter.log

jmeter.threads.JMeterThread: The delay timer was interrupted - probably did
not wait as long as intended

Those messages should be suppressed if 'running' is false.

As a result, all threads blocked waiting to sample() will get released at
pretty much the same time and as the JMeter iteration logic does the delay
before the sample() rather than after, it means all threads will go into
sample() at about the same time.

We've had cases where we've had to stop a production test as we're breaking
it and the shutdown exacerbates the problem.

As the JMeterThread.stop() sets the running flag to false, it's probably a
simple fix to wrap the code following the delay() call with if (running) {

}

May have to do a bit more than that in order to complete transaction samplers.

[asfimport]

Antony Bowesman (migrated from Bugzilla):
You're right with more needed, I just didn't understand the extra logic it was going through.

I originally assumed that there would be a public boolean isRunning() method on the JMeterThread instance and was surprised to find there wasn't. Would this be a chance to expose state like that, as it would allow, particularly Java, samplers to also check state.

[asfimport]

@pmouawad (migrated from Bugzilla):
Author: pmouawad
Date: Mon Aug 7 20:49:23 2017
New Revision: 1804356

URL: http://svn.apache.org/viewvc?rev=1804356&view=rev
Log:
#4459 - JMeter shutdown using timers releases thundering herd of interrupted samplers
Within this bug, improve fix for #3599 which does not handle correctly the shutdown.
#4459

Modified:
jmeter/trunk/src/core/org/apache/jmeter/threads/JMeterThread.java
jmeter/trunk/xdocs/changes.xml

[asfimport]

@pmouawad (migrated from Bugzilla):
Hello,
Can you try nightly build tomorrow and give your feedback ?
http://jmeter.apache.org/nightly.html
Thanks

[asfimport]

Antony Bowesman (migrated from Bugzilla):
Tested the 3.1 build and the 3.2 nightly build, ref apache-jmeter-r1804400 and it fixed the problem.

Thanks!