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 <jbert...@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 <i...@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
> >
>

Reply via email to