Race condition in JMS_TESTS.xml

[Vladimir Sitnikov <si...@gmail.com>]

Hi,

It looks like there's a race condition in JMS_TESTS.xml.
I think it causes batch JMS_TESTS.jmx to fail, and I think this failure has
nothing to do with Gradle patch.
CI: https://travis-ci.org/apache/jmeter/jobs/538080316#L1432

"setUp Thread Group" starts JMS server
Then JMS tests are performed
"tearDown Thread Group" shuts down the JMS server

Then notifyTestListenersOfEnd is called, and it seems to cause
"JMSException: Cannot send, channel has already failed".
Note: the JMS server has already been closed by that point, so it is not
clear which sense does it make to close connections then.

The call sequence is as follows
StandardJMeterEngine.notifyTestListenersOfEnd ->
jms.sampler.PublisherSampler.testEnded -> jms.client.ClientPool.clearClient
-> ActiveMQMessageProducer.close

Theoretically speaking, the solution is to move "JMS server shutdown"
*after* testEnded event, however it looks like there's no such a feature
yet.

It looks like somebody has thought of that scenario, and I see a 5sec delay
before jms shutdown.
However it does not really help: JMeter waits till the finish of teardown
group anyway, so that 5 sec delay just increases test duration, and it has
nothing to do with preventing ClientPool.clearClient and jms shutdown
racing.

Any thoughts on that?

It looks like the teardown in question needs to be reworked to use a new
thread which would postpone JMS server shutdown.
That is something behind the lines of

import org.apache.activemq.broker.BrokerService;
import org.apache.jmeter.util.JMeterUtils;
import org.apache.commons.io.FileUtils;

BrokerService broker = props.get("ACTIVEMQ_BROKER");
new Thread(() -> {
    Thread.sleep(1000);
    broker.stop();
    FileUtils.deleteDirectory(new File(JMeterUtils.getJMeterHome(),
"bin/activemq-data"));
}).start()


Vladimir

Re: Race condition in JMS_TESTS.xml

[Vladimir Sitnikov <si...@gmail.com>]

sebb>But does the error occur in the Jenkins tests?
sebb>they may well interfere in other tests in subtle ways.

Please feel free to investigate.

Vladimir

Re: Race condition in JMS_TESTS.xml

[sebb <se...@gmail.com>]

On Tue, 28 May 2019 at 09:10, Vladimir Sitnikov
<si...@gmail.com> wrote:
>
> sebb>If not, why not?
>
> There's an "obvious" flaw in the test logic, so the answer/investigation is
> not really important.

But does the error occur in the Jenkins tests?

> Probably it comes from the fact that Gradle executes batch tests
> concurrently with other build activities.

AFAIK, the tests are designed to be run one at a time.

I don't know what the 'other' build activities are, but it seems to me
they may well interfere in other tests in subtle ways.

> Vladimir

Re: Race condition in JMS_TESTS.xml

[Vladimir Sitnikov <si...@gmail.com>]

Here's the fix I have in mind:
https://github.com/apache/jmeter/pull/464/files
Please let me know if there are other/better approaches.
Otherwise I commit the workaround.

Vladimir

Re: Race condition in JMS_TESTS.xml

[Vladimir Sitnikov <si...@gmail.com>]

sebb>If not, why not?

There's an "obvious" flaw in the test logic, so the answer/investigation is
not really important.
Probably it comes from the fact that Gradle executes batch tests
concurrently with other build activities.

Vladimir

Re: Race condition in JMS_TESTS.xml

[sebb <se...@gmail.com>]

On Tue, 28 May 2019 at 08:30, Vladimir Sitnikov
<si...@gmail.com> wrote:
>
> Hi,
>
> It looks like there's a race condition in JMS_TESTS.xml.
> I think it causes batch JMS_TESTS.jmx to fail, and I think this failure has
> nothing to do with Gradle patch.
> CI: https://travis-ci.org/apache/jmeter/jobs/538080316#L1432

Has it ever failed on Jenkins, i.e. using Ant?
If not, why not?

> "setUp Thread Group" starts JMS server
> Then JMS tests are performed
> "tearDown Thread Group" shuts down the JMS server
>
> Then notifyTestListenersOfEnd is called, and it seems to cause
> "JMSException: Cannot send, channel has already failed".
> Note: the JMS server has already been closed by that point, so it is not
> clear which sense does it make to close connections then.
>
> The call sequence is as follows
> StandardJMeterEngine.notifyTestListenersOfEnd ->
> jms.sampler.PublisherSampler.testEnded -> jms.client.ClientPool.clearClient
> -> ActiveMQMessageProducer.close
>
> Theoretically speaking, the solution is to move "JMS server shutdown"
> *after* testEnded event, however it looks like there's no such a feature
> yet.
>
> It looks like somebody has thought of that scenario, and I see a 5sec delay
> before jms shutdown.
> However it does not really help: JMeter waits till the finish of teardown
> group anyway, so that 5 sec delay just increases test duration, and it has
> nothing to do with preventing ClientPool.clearClient and jms shutdown
> racing.
>
> Any thoughts on that?
>
> It looks like the teardown in question needs to be reworked to use a new
> thread which would postpone JMS server shutdown.
> That is something behind the lines of
>
> import org.apache.activemq.broker.BrokerService;
> import org.apache.jmeter.util.JMeterUtils;
> import org.apache.commons.io.FileUtils;
>
> BrokerService broker = props.get("ACTIVEMQ_BROKER");
> new Thread(() -> {
>     Thread.sleep(1000);
>     broker.stop();
>     FileUtils.deleteDirectory(new File(JMeterUtils.getJMeterHome(),
> "bin/activemq-data"));
> }).start()
>
>
> Vladimir