Add support for configuring Pulsar listener container concurrency #42062

vpavic · 2024-08-30T05:06:24Z

This commit adds configuration property that allows users to configure Pulsar message listener container concurrency.

In similar fashion to #42052, this is much needed to achieve optimal setup of message consumption, especially when dealing with partitioned topics.

/cc @onobc

onobc

Thanks for the contribution @vpavic . I believe this is another property that makes sense to make available via config props.

I have a couple of comments/suggestions to follow up on.

Thanks

onobc · 2024-08-30T15:00:59Z

...oconfigure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarProperties.java

@@ -801,6 +802,11 @@ public static class Listener {
 		 */
 		private SchemaType schemaType;

+		/**
+		 * Number of threads used by listener container.


It may be helpful to elaborate on this one just a bit, maybe something like:

Number of listener threads used by the container to process messages concurrently

Wdyt?

I tried to aligned property description with these:

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java

Lines 976 to 979 in c06027b

/**

* Number of threads to run in the listener containers.

*/

private Integer concurrency;

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jms/JmsProperties.java

Lines 171 to 180 in c06027b

/**

* Minimum number of concurrent consumers. When max-concurrency is not specified

* the minimum will also be used as the maximum.

*/

private Integer minConcurrency;

/**

* Maximum number of concurrent consumers.

*/

private Integer maxConcurrency;

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java

Lines 852 to 860 in c06027b

/**

* Minimum number of listener invoker threads.

*/

private Integer concurrency;

/**

* Maximum number of listener invoker threads.

*/

private Integer maxConcurrency;

As a user I'd probably prefer consistency in describing the same concept in different parts of auto-configuration, but whatever Spring Boot team prefers here is fine with me.

tried to aligned property description with these

Yep, I saw those descriptions as well. I think there is room for improvement in the spring-kafka properties. This is really a nit and consistency is a good thing. As you said, let's see what Boot peepz think.

onobc · 2024-08-30T15:21:16Z

...gure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarPropertiesMapper.java

@@ -195,6 +197,13 @@ private void customizePulsarContainerListenerProperties(PulsarContainerPropertie
 		map.from(properties::isObservationEnabled).to(containerProperties::setObservationEnabled);
 	}

+	<T> void customizeConcurrentPulsarListenerContainerFactory(


I just noticed that the listener containers on the framework side are inconsistent w/ the concurrency attribute between reactive and imperative. I created this issue to simplify the latter.
If you wanted to handle that issue first it would then simplify this PR so that we would not need this new customizeConcurrentPulsarListenerContainerFactory method and could simply map the property in customizePulsarContainerListenerProperties instead.

Sure, I can pick up that issue on Spring Pulsar side. So basically this PR is then blocked until spring-projects/spring-pulsar#820 is resolved.

Sure, I can pick up that issue on Spring Pulsar side. So basically this PR is then blocker until spring-projects/spring-pulsar#820 is resolved.

Thanks @vpavic . Yeh, I think we should do the spring-pulsar one first.

...oconfigure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarProperties.java

This commit adds configuration property that allows users to configure Pulsar message listener container concurrency.

Add a configuration property that allows users to configure Pulsar message listener container concurrency. See gh-42062

philwebb · 2024-09-04T04:05:33Z

Thanks @vpavic!

This is a follow-up to spring-projectsgh-42062 that utilizes newly introduced `concurrency` property in `PulsarContainerProperties` to simplify auto-configuration support for Pulsar listener container concurrency. See: spring-projects/spring-pulsar#820

vpavic · 2024-09-04T10:26:48Z

@philwebb this was merged perhaps too soon as I didn't have chance to update this PR to benefit from the changes introduced in spring-projects/spring-pulsar#820.

I've opened #42120 to address that separately.

This is a follow-up to spring-projectsgh-42062 that utilizes newly introduced `concurrency` property in `PulsarContainerProperties` to simplify auto-configuration support for Pulsar listener container concurrency. See: spring-projects/spring-pulsar#820

This is a follow-up to gh-42062 that utilizes newly introduced `concurrency` property in `PulsarContainerProperties` to simplify auto-configuration support for Pulsar listener container concurrency. See: spring-projects/spring-pulsar#820 See gh-42120

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 30, 2024

onobc suggested changes Aug 30, 2024

View reviewed changes

philwebb added the status: waiting-for-feedback We need additional information before we can continue label Aug 30, 2024

Add support for configuring Pulsar listener container concurrency

7601951

This commit adds configuration property that allows users to configure Pulsar message listener container concurrency.

vpavic force-pushed the pulsar-listener-concurrency branch from 01e745e to 7601951 Compare September 2, 2024 15:17

vpavic mentioned this pull request Sep 2, 2024

Listener concurrency attribute is inconsistent spring-projects/spring-pulsar#820

Closed

philwebb added type: enhancement A general enhancement and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Sep 3, 2024

philwebb added this to the 3.4.x milestone Sep 3, 2024

philwebb pushed a commit that referenced this pull request Sep 4, 2024

Add support for configuring Pulsar listener container concurrency

ddf7af7

Add a configuration property that allows users to configure Pulsar message listener container concurrency. See gh-42062

philwebb closed this in 3570c69 Sep 4, 2024

philwebb modified the milestones: 3.4.x, 3.4.0-M3 Sep 4, 2024

vpavic deleted the pulsar-listener-concurrency branch September 4, 2024 08:29

vpavic mentioned this pull request Sep 4, 2024

Improve Pulsar listener container concurrency configuration #42120

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for configuring Pulsar listener container concurrency #42062

Add support for configuring Pulsar listener container concurrency #42062

vpavic commented Aug 30, 2024

onobc left a comment

onobc Aug 30, 2024

vpavic Sep 2, 2024

onobc Sep 2, 2024

onobc Aug 30, 2024

vpavic Sep 2, 2024 •

edited

Loading

onobc Sep 2, 2024

philwebb commented Sep 4, 2024

vpavic commented Sep 4, 2024

	/**
	* Number of threads to run in the listener containers.
	*/
	private Integer concurrency;

	/**
	* Minimum number of concurrent consumers. When max-concurrency is not specified
	* the minimum will also be used as the maximum.
	*/
	private Integer minConcurrency;

	/**
	* Maximum number of concurrent consumers.
	*/
	private Integer maxConcurrency;

	/**
	* Minimum number of listener invoker threads.
	*/
	private Integer concurrency;

	/**
	* Maximum number of listener invoker threads.
	*/
	private Integer maxConcurrency;

Add support for configuring Pulsar listener container concurrency #42062

Add support for configuring Pulsar listener container concurrency #42062

Conversation

vpavic commented Aug 30, 2024

onobc left a comment

Choose a reason for hiding this comment

onobc Aug 30, 2024

Choose a reason for hiding this comment

vpavic Sep 2, 2024

Choose a reason for hiding this comment

onobc Sep 2, 2024

Choose a reason for hiding this comment

onobc Aug 30, 2024

Choose a reason for hiding this comment

vpavic Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

onobc Sep 2, 2024

Choose a reason for hiding this comment

philwebb commented Sep 4, 2024

vpavic commented Sep 4, 2024

vpavic Sep 2, 2024 •

edited

Loading