You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@activemq.apache.org by Justin Bertram <jb...@apache.org> on 2020/12/03 03:31:38 UTC

Re: How to cleanly close embedded ActiveMQ Artemis

I loaded JavaLite into my IDE and reproduced some test failures. Here are
my observations...

One thing that I noticed is that it's possible for some of your tests to
leak brokers. Any test that has an assertion before the broker is stopped
can leak if that assertion fails because the test will terminate without
stopping the broker. This will negatively impact any tests that follow. You
should stop your brokers in a finally block or perhaps in the @After
method. In any case, you need to make absolutely sure that no matter what
happens in the test the broker is stopped.

Your tests also leak journals. You create the embedded broker's journal
directory using this:

    Files.createTempDirectory("async").toFile().getCanonicalPath();

However, you never clean that directory up. I ran thousands of test
iterations in a loop and it filled up over 200GB of disk space.

I think the most important thing is that when you send messages in your
tests you send them as non-persistent which means they will be sent
*asynchronously*. However, your tests don't take this into account which
can lead to race conditions and ultimately assertion/test failures. I see a
couple of options to resolve this. You could send messages as persistent
which will be done synchronously. You could set
"blockOnNonDurableSend=true" on your embedded client's URL (discussed in
the documentation [1]) which will also make sending the messages
synchronous. Or you could add some other kind of Wait.waitFor() to ensure a
meaningful condition is met before proceeding with the test (although you
won't be able to inspect the message-count in most tests since you're using
asynchronous message listeners).

I also think your tests would be more robust if you used Wait.waitFor()
when inspecting HelloCommand.counter() and if you did this *before* you
shut down the broker. Given the asynchronous nature of the message
listeners there may be a discrepancy between the counter's value and the
queue's message count for a short time. Furthermore, leaving the broker up
during the Wait.waitFor() will mean that the message listeners don't get
prematurely stopped.

When I first started running the tests I could consistently reproduce a
failure in less than 25 runs or so. With the changes I listed above I was
able to run over 12,000 times without a failure.

Ultimately I don't see any issues with the broker at this point, just
issues with the tests.


Justin

[1]
http://activemq.apache.org/components/artemis/documentation/latest/send-guarantees.html#non-transactional-message-sends

On Mon, Nov 30, 2020 at 4:05 PM Igor Polevoy <ig...@javalite.io> wrote:

> HI, all.
>
> The question is posted to SO here:
>
>
> https://stackoverflow.com/questions/64991366/how-to-cleanly-close-embedded-activemq-artemis
>
> I'd like to add more details. This error is only detected on some computers
> only during JavaLite tests, and only recently. Nonetheless, it might expose
> an issue.
>
> It happens during execution of this test (others too):
>
> https://github.com/javalite/javalite/blob/java8/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java
>
> As you can see, the broker starts  and stops in the context of every test
> method.
>
> Within each test, we start a new broker, ensure that messages sent,
> accounted for, and then we stop the broker.
>
> Here is an example:
>
> This test sends 100 messages:
>
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L101
>
> This test sends 2 messages:
>
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L148
>
> They both fail due to the incorrect number of messages tested at the end.
> The 2 messages test fails because it receives 0 messages, and the 100
> message test fails because it receives 102 messages. This is despite the
> fact that every test has a new broker located which uses a new temporary
> directory that is created anew at the start and deleted at  the end of
> every test.
>
> This is not a problem with a real production since we do not expect
> multiple embedded brokers in the same JVM instance. Nonetheless, it might
> be an issue in Artemis that manifests itself like this. We are moving
> JavaLite to a new CI server, and this is a show stopper, as the build fails
> there with some random conditions (different tests fail randomly).
>
> Any tests executed individually  by Maven on command line suceeds without
> issues, which makes me believe Artemis might leak some data in JVM across
> multiple instances.
>
>
> *Note*: if you want to follow JavaLite code, please ensure to look at the
> java8 branch,
>
>
> Any help is much appreciated.
>
> Igor
>

Re: How to cleanly close embedded ActiveMQ Artemis

Posted by Igor Polevoy <ig...@javalite.io>.
Hi, Justin.

I followed your advice and achieved a reproducible successful build. In
years running  this test, we never had any issues,  probably because a
considerable hardware capacity was masking  race conditions  in code.

What really piqued my attention is the fact that in case messages are not
persistent, then the act of sending is asynchronous - good to know. In any
case, armed with this knowledge, I made the changes  in code that
prevented any possible race conditions when starting/stopping embedded
brokers multiple times in the same JVM and that  solved the problem.

Appreciate your help! Feel free to post the answer on SO, and I will mark
it as correct and will upvote it.

Thanks,
Igor



On Thu, Dec 3, 2020 at 10:11 AM Justin Bertram <jb...@apache.org> wrote:

> Please let me know how it turns out.
>
>
> Justin
>
> On Thu, Dec 3, 2020 at 8:33 AM Igor Polevoy <ig...@javalite.io> wrote:
>
> > Hi, Justin!
> >
> > *First*, appreciate you taking time looking into this!
> > *Second*, glad the issue is not with the broker.
> > *Third*, the work is done on the java8 branch, where the journals
> directory
> > is created like this in the @Before method:
> >
> > asyncRoot =
> > Files.createTempDirectory(UUID.randomUUID().toString()).toFile()
> > .getCanonicalPath();
> >
> > and cleaned after each test in the @After method:
> >
> >      Util.recursiveDelete(Paths.get(asyncRoot));
> >
> > However, I will  follow your suggestions to make the corresponding
> changes
> > to the tests to fix the problem.
> >
> > Appreciate the effort again!
> > igor
> >
> >
> > On Wed, Dec 2, 2020 at 9:32 PM Justin Bertram <jb...@apache.org>
> wrote:
> >
> > > I loaded JavaLite into my IDE and reproduced some test failures. Here
> are
> > > my observations...
> > >
> > > One thing that I noticed is that it's possible for some of your tests
> to
> > > leak brokers. Any test that has an assertion before the broker is
> stopped
> > > can leak if that assertion fails because the test will terminate
> without
> > > stopping the broker. This will negatively impact any tests that follow.
> > You
> > > should stop your brokers in a finally block or perhaps in the @After
> > > method. In any case, you need to make absolutely sure that no matter
> what
> > > happens in the test the broker is stopped.
> > >
> > > Your tests also leak journals. You create the embedded broker's journal
> > > directory using this:
> > >
> > >     Files.createTempDirectory("async").toFile().getCanonicalPath();
> > >
> > > However, you never clean that directory up. I ran thousands of test
> > > iterations in a loop and it filled up over 200GB of disk space.
> > >
> > > I think the most important thing is that when you send messages in your
> > > tests you send them as non-persistent which means they will be sent
> > > *asynchronously*. However, your tests don't take this into account
> which
> > > can lead to race conditions and ultimately assertion/test failures. I
> > see a
> > > couple of options to resolve this. You could send messages as
> persistent
> > > which will be done synchronously. You could set
> > > "blockOnNonDurableSend=true" on your embedded client's URL (discussed
> in
> > > the documentation [1]) which will also make sending the messages
> > > synchronous. Or you could add some other kind of Wait.waitFor() to
> > ensure a
> > > meaningful condition is met before proceeding with the test (although
> you
> > > won't be able to inspect the message-count in most tests since you're
> > using
> > > asynchronous message listeners).
> > >
> > > I also think your tests would be more robust if you used Wait.waitFor()
> > > when inspecting HelloCommand.counter() and if you did this *before* you
> > > shut down the broker. Given the asynchronous nature of the message
> > > listeners there may be a discrepancy between the counter's value and
> the
> > > queue's message count for a short time. Furthermore, leaving the broker
> > up
> > > during the Wait.waitFor() will mean that the message listeners don't
> get
> > > prematurely stopped.
> > >
> > > When I first started running the tests I could consistently reproduce a
> > > failure in less than 25 runs or so. With the changes I listed above I
> was
> > > able to run over 12,000 times without a failure.
> > >
> > > Ultimately I don't see any issues with the broker at this point, just
> > > issues with the tests.
> > >
> > >
> > > Justin
> > >
> > > [1]
> > >
> > >
> >
> http://activemq.apache.org/components/artemis/documentation/latest/send-guarantees.html#non-transactional-message-sends
> > >
> > > On Mon, Nov 30, 2020 at 4:05 PM Igor Polevoy <ig...@javalite.io> wrote:
> > >
> > > > HI, all.
> > > >
> > > > The question is posted to SO here:
> > > >
> > > >
> > > >
> > >
> >
> https://stackoverflow.com/questions/64991366/how-to-cleanly-close-embedded-activemq-artemis
> > > >
> > > > I'd like to add more details. This error is only detected on some
> > > computers
> > > > only during JavaLite tests, and only recently. Nonetheless, it might
> > > expose
> > > > an issue.
> > > >
> > > > It happens during execution of this test (others too):
> > > >
> > > >
> > >
> >
> https://github.com/javalite/javalite/blob/java8/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java
> > > >
> > > > As you can see, the broker starts  and stops in the context of every
> > test
> > > > method.
> > > >
> > > > Within each test, we start a new broker, ensure that messages sent,
> > > > accounted for, and then we stop the broker.
> > > >
> > > > Here is an example:
> > > >
> > > > This test sends 100 messages:
> > > >
> > > >
> > >
> >
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L101
> > > >
> > > > This test sends 2 messages:
> > > >
> > > >
> > >
> >
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L148
> > > >
> > > > They both fail due to the incorrect number of messages tested at the
> > end.
> > > > The 2 messages test fails because it receives 0 messages, and the 100
> > > > message test fails because it receives 102 messages. This is despite
> > the
> > > > fact that every test has a new broker located which uses a new
> > temporary
> > > > directory that is created anew at the start and deleted at  the end
> of
> > > > every test.
> > > >
> > > > This is not a problem with a real production since we do not expect
> > > > multiple embedded brokers in the same JVM instance. Nonetheless, it
> > might
> > > > be an issue in Artemis that manifests itself like this. We are moving
> > > > JavaLite to a new CI server, and this is a show stopper, as the build
> > > fails
> > > > there with some random conditions (different tests fail randomly).
> > > >
> > > > Any tests executed individually  by Maven on command line suceeds
> > without
> > > > issues, which makes me believe Artemis might leak some data in JVM
> > across
> > > > multiple instances.
> > > >
> > > >
> > > > *Note*: if you want to follow JavaLite code, please ensure to look at
> > the
> > > > java8 branch,
> > > >
> > > >
> > > > Any help is much appreciated.
> > > >
> > > > Igor
> > > >
> > >
> >
>

Re: How to cleanly close embedded ActiveMQ Artemis

Posted by Justin Bertram <jb...@apache.org>.
Please let me know how it turns out.


Justin

On Thu, Dec 3, 2020 at 8:33 AM Igor Polevoy <ig...@javalite.io> wrote:

> Hi, Justin!
>
> *First*, appreciate you taking time looking into this!
> *Second*, glad the issue is not with the broker.
> *Third*, the work is done on the java8 branch, where the journals directory
> is created like this in the @Before method:
>
> asyncRoot =
> Files.createTempDirectory(UUID.randomUUID().toString()).toFile()
> .getCanonicalPath();
>
> and cleaned after each test in the @After method:
>
>      Util.recursiveDelete(Paths.get(asyncRoot));
>
> However, I will  follow your suggestions to make the corresponding changes
> to the tests to fix the problem.
>
> Appreciate the effort again!
> igor
>
>
> On Wed, Dec 2, 2020 at 9:32 PM Justin Bertram <jb...@apache.org> wrote:
>
> > I loaded JavaLite into my IDE and reproduced some test failures. Here are
> > my observations...
> >
> > One thing that I noticed is that it's possible for some of your tests to
> > leak brokers. Any test that has an assertion before the broker is stopped
> > can leak if that assertion fails because the test will terminate without
> > stopping the broker. This will negatively impact any tests that follow.
> You
> > should stop your brokers in a finally block or perhaps in the @After
> > method. In any case, you need to make absolutely sure that no matter what
> > happens in the test the broker is stopped.
> >
> > Your tests also leak journals. You create the embedded broker's journal
> > directory using this:
> >
> >     Files.createTempDirectory("async").toFile().getCanonicalPath();
> >
> > However, you never clean that directory up. I ran thousands of test
> > iterations in a loop and it filled up over 200GB of disk space.
> >
> > I think the most important thing is that when you send messages in your
> > tests you send them as non-persistent which means they will be sent
> > *asynchronously*. However, your tests don't take this into account which
> > can lead to race conditions and ultimately assertion/test failures. I
> see a
> > couple of options to resolve this. You could send messages as persistent
> > which will be done synchronously. You could set
> > "blockOnNonDurableSend=true" on your embedded client's URL (discussed in
> > the documentation [1]) which will also make sending the messages
> > synchronous. Or you could add some other kind of Wait.waitFor() to
> ensure a
> > meaningful condition is met before proceeding with the test (although you
> > won't be able to inspect the message-count in most tests since you're
> using
> > asynchronous message listeners).
> >
> > I also think your tests would be more robust if you used Wait.waitFor()
> > when inspecting HelloCommand.counter() and if you did this *before* you
> > shut down the broker. Given the asynchronous nature of the message
> > listeners there may be a discrepancy between the counter's value and the
> > queue's message count for a short time. Furthermore, leaving the broker
> up
> > during the Wait.waitFor() will mean that the message listeners don't get
> > prematurely stopped.
> >
> > When I first started running the tests I could consistently reproduce a
> > failure in less than 25 runs or so. With the changes I listed above I was
> > able to run over 12,000 times without a failure.
> >
> > Ultimately I don't see any issues with the broker at this point, just
> > issues with the tests.
> >
> >
> > Justin
> >
> > [1]
> >
> >
> http://activemq.apache.org/components/artemis/documentation/latest/send-guarantees.html#non-transactional-message-sends
> >
> > On Mon, Nov 30, 2020 at 4:05 PM Igor Polevoy <ig...@javalite.io> wrote:
> >
> > > HI, all.
> > >
> > > The question is posted to SO here:
> > >
> > >
> > >
> >
> https://stackoverflow.com/questions/64991366/how-to-cleanly-close-embedded-activemq-artemis
> > >
> > > I'd like to add more details. This error is only detected on some
> > computers
> > > only during JavaLite tests, and only recently. Nonetheless, it might
> > expose
> > > an issue.
> > >
> > > It happens during execution of this test (others too):
> > >
> > >
> >
> https://github.com/javalite/javalite/blob/java8/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java
> > >
> > > As you can see, the broker starts  and stops in the context of every
> test
> > > method.
> > >
> > > Within each test, we start a new broker, ensure that messages sent,
> > > accounted for, and then we stop the broker.
> > >
> > > Here is an example:
> > >
> > > This test sends 100 messages:
> > >
> > >
> >
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L101
> > >
> > > This test sends 2 messages:
> > >
> > >
> >
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L148
> > >
> > > They both fail due to the incorrect number of messages tested at the
> end.
> > > The 2 messages test fails because it receives 0 messages, and the 100
> > > message test fails because it receives 102 messages. This is despite
> the
> > > fact that every test has a new broker located which uses a new
> temporary
> > > directory that is created anew at the start and deleted at  the end of
> > > every test.
> > >
> > > This is not a problem with a real production since we do not expect
> > > multiple embedded brokers in the same JVM instance. Nonetheless, it
> might
> > > be an issue in Artemis that manifests itself like this. We are moving
> > > JavaLite to a new CI server, and this is a show stopper, as the build
> > fails
> > > there with some random conditions (different tests fail randomly).
> > >
> > > Any tests executed individually  by Maven on command line suceeds
> without
> > > issues, which makes me believe Artemis might leak some data in JVM
> across
> > > multiple instances.
> > >
> > >
> > > *Note*: if you want to follow JavaLite code, please ensure to look at
> the
> > > java8 branch,
> > >
> > >
> > > Any help is much appreciated.
> > >
> > > Igor
> > >
> >
>

Re: How to cleanly close embedded ActiveMQ Artemis

Posted by Igor Polevoy <ig...@javalite.io>.
Hi, Justin!

*First*, appreciate you taking time looking into this!
*Second*, glad the issue is not with the broker.
*Third*, the work is done on the java8 branch, where the journals directory
is created like this in the @Before method:

asyncRoot = Files.createTempDirectory(UUID.randomUUID().toString()).toFile()
.getCanonicalPath();

and cleaned after each test in the @After method:

     Util.recursiveDelete(Paths.get(asyncRoot));

However, I will  follow your suggestions to make the corresponding changes
to the tests to fix the problem.

Appreciate the effort again!
igor


On Wed, Dec 2, 2020 at 9:32 PM Justin Bertram <jb...@apache.org> wrote:

> I loaded JavaLite into my IDE and reproduced some test failures. Here are
> my observations...
>
> One thing that I noticed is that it's possible for some of your tests to
> leak brokers. Any test that has an assertion before the broker is stopped
> can leak if that assertion fails because the test will terminate without
> stopping the broker. This will negatively impact any tests that follow. You
> should stop your brokers in a finally block or perhaps in the @After
> method. In any case, you need to make absolutely sure that no matter what
> happens in the test the broker is stopped.
>
> Your tests also leak journals. You create the embedded broker's journal
> directory using this:
>
>     Files.createTempDirectory("async").toFile().getCanonicalPath();
>
> However, you never clean that directory up. I ran thousands of test
> iterations in a loop and it filled up over 200GB of disk space.
>
> I think the most important thing is that when you send messages in your
> tests you send them as non-persistent which means they will be sent
> *asynchronously*. However, your tests don't take this into account which
> can lead to race conditions and ultimately assertion/test failures. I see a
> couple of options to resolve this. You could send messages as persistent
> which will be done synchronously. You could set
> "blockOnNonDurableSend=true" on your embedded client's URL (discussed in
> the documentation [1]) which will also make sending the messages
> synchronous. Or you could add some other kind of Wait.waitFor() to ensure a
> meaningful condition is met before proceeding with the test (although you
> won't be able to inspect the message-count in most tests since you're using
> asynchronous message listeners).
>
> I also think your tests would be more robust if you used Wait.waitFor()
> when inspecting HelloCommand.counter() and if you did this *before* you
> shut down the broker. Given the asynchronous nature of the message
> listeners there may be a discrepancy between the counter's value and the
> queue's message count for a short time. Furthermore, leaving the broker up
> during the Wait.waitFor() will mean that the message listeners don't get
> prematurely stopped.
>
> When I first started running the tests I could consistently reproduce a
> failure in less than 25 runs or so. With the changes I listed above I was
> able to run over 12,000 times without a failure.
>
> Ultimately I don't see any issues with the broker at this point, just
> issues with the tests.
>
>
> Justin
>
> [1]
>
> http://activemq.apache.org/components/artemis/documentation/latest/send-guarantees.html#non-transactional-message-sends
>
> On Mon, Nov 30, 2020 at 4:05 PM Igor Polevoy <ig...@javalite.io> wrote:
>
> > HI, all.
> >
> > The question is posted to SO here:
> >
> >
> >
> https://stackoverflow.com/questions/64991366/how-to-cleanly-close-embedded-activemq-artemis
> >
> > I'd like to add more details. This error is only detected on some
> computers
> > only during JavaLite tests, and only recently. Nonetheless, it might
> expose
> > an issue.
> >
> > It happens during execution of this test (others too):
> >
> >
> https://github.com/javalite/javalite/blob/java8/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java
> >
> > As you can see, the broker starts  and stops in the context of every test
> > method.
> >
> > Within each test, we start a new broker, ensure that messages sent,
> > accounted for, and then we stop the broker.
> >
> > Here is an example:
> >
> > This test sends 100 messages:
> >
> >
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L101
> >
> > This test sends 2 messages:
> >
> >
> https://github.com/javalite/javalite/blob/master/javalite-async/src/test/java/org/javalite/async/AsyncSpec.java#L148
> >
> > They both fail due to the incorrect number of messages tested at the end.
> > The 2 messages test fails because it receives 0 messages, and the 100
> > message test fails because it receives 102 messages. This is despite the
> > fact that every test has a new broker located which uses a new temporary
> > directory that is created anew at the start and deleted at  the end of
> > every test.
> >
> > This is not a problem with a real production since we do not expect
> > multiple embedded brokers in the same JVM instance. Nonetheless, it might
> > be an issue in Artemis that manifests itself like this. We are moving
> > JavaLite to a new CI server, and this is a show stopper, as the build
> fails
> > there with some random conditions (different tests fail randomly).
> >
> > Any tests executed individually  by Maven on command line suceeds without
> > issues, which makes me believe Artemis might leak some data in JVM across
> > multiple instances.
> >
> >
> > *Note*: if you want to follow JavaLite code, please ensure to look at the
> > java8 branch,
> >
> >
> > Any help is much appreciated.
> >
> > Igor
> >
>