Update JMS listener concurrency configuration to set the same minimum and maximum number of consumers when users specify only the minimum using spring.jms.listener.concurrency property.
Prior to this commit, when using spring.jms.listener.concurrency to set the minimum number of consumers without also specifying spring.jms.listener.max-concurrency would result in effective concurrency where the actual minimum number of consumers is always 1, while the maximum number of consumers is the value of spring.jms.listener.concurrency.
This fix results in a breaking change, however the current behavior is simply not aligned with the description of spring.jms.listener.concurrency property.
Also see the javadoc of DefaultMessageListenerContainer#setConcurrency which states:
Specify concurrency limits via a "lower-upper" String, e.g. "5-10", or a simple upper limit String, e.g. "10" (the lower limit will be 1 in this case).
Comment From: wilkinsona
Thanks, @vpavic.
I'm tempted to rework the properties a little bit as well. Given the setConcurrency method that takes a String, the spring.jms.listener.concurrency perhaps sets the expectation that it should take a String with the same syntax. To make it clearer that it does not, I wonder if we should rename things. The ideal would be:
spring.jms.listener.concurrency.minspring.jms.listener.concurrency.max
This is tricky due to the clash with the existing spring.jms.listener.concurrency property. We may have to settle for something like this instead:
spring.jms.listener.min-concurrencyspring.jms.listener.max-concurrency
I also wonder if spring.jms.listener.min-concurrency should default to 1. This would make make the "simple upper limit" case explicit such that if you only set max-concurrency we always configure a "lower-upper" string that's 1-n.
There's no need for any updates to the changes proposed here. I'd like to discuss the above with the team so we can decide if and when to change things. It may be that the changes are made across different releases.
Comment From: vpavic
Your spring.jms.listener.min-concurrency proposal (plus the deprecation of spring.jms.listener.concurrency I assume) seems like the least impactful to developers (because it doesn't change the max property) while also making things more explicit and clear.
If the team decides that's the way forward, I can put together a separate PR for that change as well.
Comment From: wilkinsona
Thanks very much, @vpavic.