cshannon commented on PR #928: URL: https://github.com/apache/activemq/pull/928#issuecomment-1299090316
@mattrpav , @jbonofre . @lucastetreault - This is ready for review, see what you think. Part of this commit reverts the previous change and updates with a new test. Because technically ConsumerInfos can be associated with multiple Subscriptions and because ConsumerInfo objects can change if a durable subscription goes offline and back online I thought it was generally a bad idea to store a map associating them as you'd have to keep it up to date on changes with durables (memory leak in the original PR because of that) and doesn't really work for one to many association of composite destinations even if rarely used. The new approach here is to just use the existing Regions that already handle all of that and track in a concurrent map consumer ids and subscriptions so all we have to do is look through the region or regions (if composite) to grab the subs and then we are good. We shouldn't need it but I added a fail safe where it falls back on Exception or if it couldn't find a sub using the new method but that shouldn't happen unless something went very wrong. I would expect performance to still be much better than before and similar to the previous commit as we are still just looking up the subscriptions in concurrent maps vs iterating. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
