awslabs / awslabs/amazon-sqs-java-messaging-lib
Memory leak on retry cases when using UNORDERED_ACKNOWLEDGE mode
- Dominant language
- Java
- Stars
- 183
- Forks
- 150
- PR merge metrics
- No merged PRs in 30d
Description
Hey,
we have basically the same issue as described [here](https://github.com/awslabs/amazon-sqs-java-messaging-lib/issues/37) - we're also using Spring JMS with AWS JMS library.
We're using `UNORDERED_ACKNOWLEDGE` mode to be able to apply custom retry policies setting the visibility timeout on a message basis. This worked pretty well for us but we could see memory leaks appearing over time. After analyzing we could see, what also the other poster was mentioning: when an exception is thrown from a listener, nobody will cleanup those messages in the `UnorderedAcknowledger`. So with each retry, an entry in the unacked messages map will be created - but not cleaned up.
The suggestion to call `SQSSession.recover()` in such cases will clear the unacked messages, but will also explicitly NACK them - with a visibility timeout of 0 - which means direct retries. This was why we went with UNORDERED in the first place to be able to customize this behavior.
Also I read in the other thread, that if we use concurrency with the listeners, that each message will get its own session and so its own `UnorderedAcknowledger`? So it won't happen if clearing messages in the acker, that there are also other messages in there in parallel that would be accidentally cleaned up as well? If this holds true, the only thing for us which would be missing is, that there is a way on `SQSMessage` to get our hands on the acker, to clear the messages in there. Another option would be to not just provide `acknowledge()` on the message, but also a way to nack/clear the message - if the `Acknowledger` should not be exposed to the outside world.
I also saw some magic happening in the `SQSSession.recover()` method that talks about keeping the order. Would this also be something we would have to consider if we _don't_ use message groups?
The only workaround we could do for now is, to use reflection to get our hands on the acker from the message and call `forgetUnAckMessages()` in case of an exception. But this is quite hacky.
Any insights or suggestions are appreciated, thanks.
Contributor guide
Assessment
This issue has not been assessed yet.