[GitHub] [activemq] cshannon commented on pull request #965: [AMQ-9217] Fix per-destination message audits in IndividualDeadLetterStrategy

["cshannon (via GitHub)" <gi...@apache.org>]

cshannon commented on PR #965:
URL: https://github.com/apache/activemq/pull/965#issuecomment-1422497968

   @mattrpav - I reworked this a bit and have a commit here as a follow on to yours that I think makes a bit more sense: https://github.com/cshannon/activemq/commit/bf30a92809d49c7857843af7fdcbc95a114640cd
   
   It simplifies things and drops the flag as I think it's just a bug that should be fixed. The more I think about it the more it just seems completely wrong to not have an audit per DLQ destination. I didn't do much with the tests so that would need to be looked at. I also wonder if 10k is too high and maybe we should lower the default because of memory usage. 
   
   The last thing is I don't really like the LRU cache we have, it's super outdated. It would be much better to use something like Caffeine where we don't need to synchronize. That being said, that's probably a bigger task as we'd need to pull in that dependency and I would think we'd just want to replace it everywhere, not just here. Also, (in theory) the DLQ stuff shouldn't be hit all the time so hopefully performance wouldn't be a big deal as I wouldn't normally expect a ton of attempts to acquire the lock at the same time but I guess it's possible if things are going really bad.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org