JMS: Tombstone the message handling on abort (#904)

[akara]

... and validate the TxEnvelope on commit/rollback - hopefully at no or very minimal performance impact.

Unfortunately, cannot recreate issue on unit-test broker. Can consistently recreate issue on same 'abort message loss' test with our internal broker.

[ennru]

What JMS broker do you use for testing? We could run a Docker image with a JMS broker instead of the embedded ActiveMQ.

[akara]

Unfortunately, it is a highly customized version of ActiveMQ on both client and broker. Not something I can make available. In debugging this issue, we need to determine whether it is the bug in that infrastructure or on the Alpakka implementation. Looks like I have to go by reasoning on this one.

[akara]

What it turns out to be is that the session stop is not stopping the session immediately in our client, but after a few message cycles. The message held by the session at the time (based on the current code) is rolled back, but as the message handling did not stop, the next message is delivered downstream. The downstream still holds the TxEnvelope for that message we rolled back and subsequently calls commit on that envelope. The current message in the session now has changed to the next message already. So the commit call actually committed the new message that should not have been committed.

This is the reason for the proposed change. Once we abort, we ensure no more messages are handled, neither commit nor rollback. This is achieved by the tombstone. Also we want to make sure we always commit on the right message. So we pass the envelope down to be checked against the current message envelope to provide that level of validation.

The only way we can try and see whether we can recreate is to move back to an older version of the ActiveMQ broker AND client library (yes, what we're using is based on a pretty old version) and see whether we can reproduce. Not 100% sure, though.

[ennru]

Ok, I understand the problem you're solving now.
I think it would be good to explain the "Tombstone" in the code somehow. How should the JMS exception thrown for an unexpected TX envelope be handled?

[akara]

Sure. Let me add these comments in the code to make it clear. Thanks!

[akara]

Just updated the code to make the envelope validation using require() so it is clear. If validation fail, we fail the source stage and signal that IllegalArgumentException to the output port. Also added comments on the tombstone, which manifests itself as a StopMessageListenerException thrown and a catch block in the message handling loop to stop consuming.

[akara]

Build is failing for unrelated issues. Unfortunately, I can't restart the build.

[ennru]

LGTM.