Make it possible to opt in to TaskExecutor auto-configuration when an Executor has already been defined

[blake-bauman]

There appears to be a bug in TaskExecutorConfiguration when creating an AsyncTaskExecutor bean, the condition is:

@ConditionalOnMissingBean(Executor.class)

instead of on AsyncTaskExecutor.

The result is, if an app creates a bean of type Executor (such as a ScheduledExecutorService), this results in Spring’s default AsyncTaskExecutor beans to not be created. The application will fail to start because of the missing beans.

This is for apps trying to use the bean generated for the configuration with the Task Execution and Scheduling feature. If an app creates a bean of type Executor, the above bean never gets created.

[blake-bauman]

I've attached a sample project which reproduced the issue. If you run it, you'll see it fails to start due to a missing AsyncTaskExecutor bean. If you comment out the ScheduledExecutorService bean, then it succeeds.

test-spring-task-executors.tar.gz

[quaff]

I think @Bean method level @ConditionalOnMissingBean should be used instead of class level @ConditionalOnMissingBean(Executor.class).

It should only back off for TaskExecutor not Executor.

My proposal: https://github.com/spring-projects/spring-boot/compare/main...quaff:patch-137?expand=1

[quaff]

Or @ConditionalOnMissingBean(value =Executor.class, ignored = ScheduledExecutorService.class) ?

It make sense since ScheduledExecutorService should be used for scheduling task only.

[snicoll]

It should only back off for TaskExecutor not Executor.

I disagree with that and this bit of the auto-configuration is a bit tricky, I reckon. This is due to the bridge we had before the executor abstraction was available in the JDK. The condition on Executor is on purpose as users may define such a bean and expects the auto-configuration to back-off.

[blake-bauman]

@quaff ScheduledExecutorService was just an example. Any bean of type Executor will cause an app to fail startup if they need the AsyncTaskExecutor.

users may define such a bean and expects the auto-configuration to back-off

@snicoll We've found the opposite. Users define a bean for other purposes but expect to still use the AsyncTaskExecutor for other tasks.

[nosan]

@blake-bauman

Users define a bean for other purposes but expect to still use the AsyncTaskExecutor for other tasks.

In this case, you can use defaultCandidate=false annotation attribute and inject your ScheduledExecutorService using the @Qualifier annotation.
Here's an example based on what you shared earlier, with a slight adjustment:

@Bean(defaultCandidate = false)
@Qualifier("myexecutor")
public ScheduledExecutorService executorService() {
    return Executors.newSingleThreadScheduledExecutor();
}

This way, you will have both the auto-configured Executor and your custom ScheduledExecutorService.

[snicoll]

@snicoll We've found the opposite. Users define a bean for other purposes but expect to still use the AsyncTaskExecutor for other tasks.

I am sure that does happen. Hopefully what follows explain in more details what I meant.

@jhoeller and I discussed this one at length as I was trying to get back the context of why we did this that way. The conditions currently aren't ideal but they aren't out of place either.

AsyncAnnotationBeanPostProcessor looks for a bean named taskExecutor and with type Executor. Many users have historically created such custom Executor and called it taskExecutor. If we changed the condition, then the auto-configuration would attempt to override the user's bean with the auto-configured one. We can't do that.

We could improve the condition to only backoff if no beans named taskExecutor or applicationTaskExecutor have been defined but that then means that if you created a bean with a different name and only want that then you'd have to exclude the auto-configuration somehow. That's not ideal either.

The auto-configuration creates builder that applies the logic of the auto-configuration so you could do something as follows:

@Bean
Executor myCustomExecutor() { ... }

@Bean
SimpleAsyncTaskExecutor taskExecutor(SimpleAsyncTaskExecutorBuilder builder) {
    return builder.build();
}

SimpleAsyncTaskExecutorBuilder and friends are created irrespective of the presence of an Executor bean for that very reason. I think if you have custom executors in the context, being explicit about things might be the best way out.

This is flagged for team meeting so we'll discuss this ASAP, and hopefully we can find another way that would improve the current situation.

[wilkinsona]

@snicoll and I discussed this today.

As things stand, the best way to retain the auto-configured AsyncTaskExecutor is to use @Bean(defaultCandidate = false) (as @nosan has described above):

@Bean(defaultCandidate = false)
@Qualifier("myexecutor")
ScheduledExecutorService executorService() {
    return Executors.newSingleThreadScheduledExecutor();
}

This requires Spring Framework 6.2 so is only possible in Boot 3.4 and later.

For Boot 3.3, you can use the builder to define your own AsyncTaskExecutor with minimal code as @snicoll has shown above. One thing to note is that the equivalent auto-configured bean would be named both taskExecutor and applicationTaskExecutor. This is significant as the names control its consumption elsewhere. Framework will use a bean named taskExecutor for @Async task processing. Boot will apply a bean named applicationTaskExecutor in a number of places including MVC, WebFlux, and JPA bootstrapping. You can retain this behavior by naming your AsyncTaskExecutor in the same manner:

@Bean(name = {"taskExector", "applicationTaskExecutor"})
SimpleAsyncTaskExecutor taskExecutor(SimpleAsyncTaskExecutorBuilder builder) {
    return builder.build();
}

We'd like to improve this. Unfortunately, it's complicated by existing Framework behavior.

Framework will look for a unique bean of a particular type. When there a multiple beans of that type, it will look for a bean of a particular type with a specific name (taskExecutor for async task execution or taskScheduler for task scheduling). If Boot were to continue auto-configuring a bean when another matching bean already existed, it would break some existing arrangements. If Boot's bean used one of the special names, it could change the bean that's used for task execution or scheduling or result in a failure due to multiple beans with the same name. Regardless of the name, it would make ambiguous any injection points that expected a single bean of that type.

A further complication here is that there is overlap between the types that can be used for task scheduling and async task execution. This overlap further hinders any attempt to infer intent from the beans that have bean defined.

To avoid problems described above, particularly when upgrading, we think any new behavior will have to be gated by a property. We're currently envisaging two boolean properties, one for task execution and one for scheduling. By setting one or both properties to true, you can then opt in to Boot auto-configuring its bean for task scheduling or task execution when it would have currently backed off.

Focusing on the task execution case as that's the subject of this issue, setting the property to true would result in Boot configuring its task executor bean, even if there's an existing Executor bean in the context. It would be named applicationTaskExecutor to ensure that it's picked up by MVC, JPA bootstrapping, etc and an AsyncConfigurer would be used to configure Framework to use it for async task execution. There are some wrinkles to consider, such as Framework only supporting a single AsyncConfigurer, but we believe that can be addressed by defining a @Primary AsyncConfigurer that delegates to the other one.

To allow this issue to keep its focus on task execution, I have opened #44806 for the task scheduling side of things.

[nosan]

I'm not sure whether it was done intentionally or not, but these changes introduce breaking changes:

Test to Reproduce:

class Gh44659ApplicationTests {

    @Test
    void taskExecutorBeanNameExists() {
        SpringApplication application = new SpringApplication(MyApplication.class);
        ConfigurableApplicationContext context = application.run();
        // 3.5.0-SNAPSHOT -> FAILED; 3.4.4 -> PASSED
        assertThatNoException().isThrownBy(() -> context.getBean("taskExecutor"));
    }

    @Test
    void taskExecutorBeanNameIsReserved() {
        // 3.5.0 -> FAILED; 3.4.4 -> PASSED
        assertThatException().isThrownBy(
                        () -> new SpringApplication(MyApplication.class, CustomTaskExecutorConfiguration.class)
                                .run())
                .withStackTraceContaining("Cannot register alias 'taskExecutor' for name 'applicationTaskExecutor'");
    }

    @Test
    void applicationTaskExecutorBeanNameIsReserved() {
        // 3.5.0-SNAPSHOT and 3.4.4 both PASSED
        assertThatException().isThrownBy(
                        () -> new SpringApplication(MyApplication.class, ApplicationTaskExecutorConfiguration.class)
                                .run())
                .withStackTraceContaining(
                        "Invalid bean definition with name 'applicationTaskExecutor' defined in class");
    }

    @SpringBootApplication
    static class MyApplication {

    }

    @Configuration(proxyBeanMethods = false)
    static class CustomTaskExecutorConfiguration {

        @Bean(defaultCandidate = false)
        Executor taskExecutor() {
            return Runnable::run;
        }

    }

    @Configuration(proxyBeanMethods = false)
    static class ApplicationTaskExecutorConfiguration {

        @Bean(defaultCandidate = false)
        Executor applicationTaskExecutor() {
            return Runnable::run;
        }

    }

}

• With the new changes, there is no taskExecutor bean in the context.
• The taskExecutor bean name is no longer reserved, while applicationTaskExecutor remains a reserved name.

This branch (main...nosan:spring-boot:44659) tries to address both issues while simplifying things a little bit.

[snicoll]

Thanks @nosan.

With the new changes, there is no taskExecutor bean in the context.

Yes, that is on purpose as we no longer require to use this name. We automatically configure async processing with the auto-configured one.

The taskExecutor bean name is no longer reserved, while applicationTaskExecutor remains a reserved name.

I am not sure what you mean by "reserved". How was it reserved before? I am looking at the example and I am not sure I understand what use case the test is trying to simulate.

[nosan]

Yes, that is on purpose as we no longer require to use this name. We automatically configure async processing with the auto-configured one.

This means that if anyone in their codebase is using beanFactory.getBean("taskExecutor"), these changes will affect them.

No bean named 'taskExecutor' available org.springframework.beans.factory.NoSuchBeanDefinitionException: No bean named 'taskExecutor' available

From my perspective, this qualifies as a breaking change. Prior to version 3.5.x, the bean was registered, but starting from version 3.5.x, it is no longer registered.

I am not sure what you mean by "reserved". How was it reserved before? I am looking at the example and I am not sure I understand what use case the test is trying to simulate.

Before these changes, it was not possible to use either taskExecutor or applicationTaskExecutor bean names while TaskExecutionAutoConfiguration is applied.

/** 3.4.4, not anymore in 3.5.0-SNAPSHOT
 * Cannot register alias 'taskExecutor' for name 'applicationTaskExecutor': Alias would override bean definition 'taskExecutor'
 * java.lang.IllegalStateException: Cannot register alias 'taskExecutor' for name 'applicationTaskExecutor': Alias would override bean definition *
 * 'taskExecutor'
 */
@Test
void taskExecutorBeanNameIsReserved() {
	new SpringApplication(MyApplication.class, TaskExecutorConfiguration.class)
			.run();
}

@Configuration(proxyBeanMethods = false)
static class TaskExecutorConfiguration {

    @Bean(autowireCandidate = false)
    Executor taskExecutor() {
        return Runnable::run;
    }

}

Although this may not seem like a significant issue, it creates potential for unpredictable configurations:

    @Test
    void taskExecutor() throws Exception {
        SpringApplication application = new SpringApplication(MyApplication.class);
        application.setDefaultProperties(Map.of("spring.task.execution.mode", "force",
                "spring.task.execution.thread-name-prefix", "auto-task-"));
        Future<String> echo = application.run().getBean(TestBean.class).echo("Hello world");
        assertThat(echo.get()).contains("auto-task-1");
    }
    @EnableAsync
    @SpringBootApplication
    @Import(TestBean.class)
    static class MyApplication {
//        @Bean(autowireCandidate = false)
//        Executor taskExecutor() {
//            return new SimpleAsyncTaskExecutor("autowired-candidate-executor");
//        }
        @Bean
        Executor customTaskExecutor() {
            return new SimpleAsyncTaskExecutor("customTaskExecutor-");
        }
    }
    static class TestBean {
        @Async
        Future<String> echo(String text) {
            return CompletableFuture.completedFuture(Thread.currentThread().getName() + " " + text);
        }

    }

If you uncomment the taskExecutor, the test will fail because the bean named taskExecutor will be used for @Async. Prior to version 3.5.0-SNAPSHOT, this name could not be used when auto-configuration was applied.

In my opinion, if developers want to use a custom task executor while keeping the auto-configured one, they should avoid using the bean names taskExecutor or applicationTaskExecutor.

I suggest reverting @Bean(name = { TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME, AsyncAnnotationBeanPostProcessor.DEFAULT_TASK_EXECUTOR_BEAN_NAME }) changes and prohibiting the use of taskExecutor or applicationTaskExecutor bean names while spring.task.execution.mode=force is being used. In that case, TaskExecutionAutoConfiguration would behave the same as in 3.4.x, while also allowing the possibility to register custom Executor beans while keeping the auto-configured one.

This also simplifies logic in TaskExecutionAutoConfiguration by removing:

	@ConditionalOnMissingBean(name = AsyncAnnotationBeanPostProcessor.DEFAULT_TASK_EXECUTOR_BEAN_NAME,
			value = AsyncConfigurer.class)
	static class AsyncConfigurerConfiguration {}

[nosan]

By registering an AsyncConfigurer, such a configuration becomes no longer feasible.

@Test
void taskExecutor() throws Exception {
    SpringApplication application = new SpringApplication(MyApplication.class);
    application.setDefaultProperties(Map.of("spring.task.execution.mode", "force",
            "spring.task.execution.thread-name-prefix", "auto-task-"));
    Future<String> echo = application.run().getBean(TestBean.class).echo("Hello world");
    assertThat(echo.get()).contains("customTaskExecutor-");
    // Expecting actual:
    // "auto-task-1 Hello world"
    // to contain:
    // "customTaskExecutor-" 
    // java.lang.AssertionError: 
    // Expecting actual:
    // "auto-task-1 Hello world"
    // to contain:
    // "customTaskExecutor-" 
}

@EnableAsync
@SpringBootApplication
@Import(TestBean.class)
static class MyApplication {

    @Bean
    @Primary
    Executor customTaskExecutor() {
        return new SimpleAsyncTaskExecutor("customTaskExecutor-");
    }

}

static class TestBean {

    @Async
    Future<String> echo(String text) {
        return CompletableFuture.completedFuture(Thread.currentThread().getName() + " " + text);
    }

}

[snicoll]

I think you might take this a little bit backwards.

This means that if anyone in their codebase is using beanFactory.getBean("taskExecutor"), these changes will affect them.

I can see that, but I am not aware we advertized that the auto-configuration would alias the (auto-configured) applicationTaskExecutor bean to that name. The only reason we did thus far is that because Spring Framework uses that name in case of fallback, and that's no longer the case with AsyncConfigurerConfiguration.

From my perspective, this qualifies as a breaking change. Prior to version 3.5.x, the bean was registered, but starting from version 3.5.x, it is no longer registered.

I am not sure I agree. We're talking about bean name. If you were relying on a bean name, applicationTaskExecutor should have been the name to rely on.

In my opinion, if developers want to use a custom task executor while keeping the auto-configured one, they should avoid using the bean names taskExecutor or applicationTaskExecutor.

We no longer rely on taskExecutor (or so I thought) so it's not a problem for new users to use that name. In fact, this makes the difference between the "application" one more clear. Your example with the primary custom executor and the one that shouldn't be autowired is a bit strange, but I guess we could relax AsyncConfigurerConfiguration to always apply when the mode is set to force. That would always set the auto-configured task executor regardless of what you have.

As you've seen this part of the codebase is quite complex and we're trying to help users by providing them an escape hatch.

In summary, I can see that your concerns are as follows:

• taskExecutor no longer registered
• unpredictible behavior, but only when the force mode is set.

Hopefully I should have provided an explanation for the first point. As for the second, this is a new feature, and therefore a new behavior so it doesn't qualify as breaking change to me. Based on your feedback, I think we must relax the condition for AsyncConfigurerConfiguration. Would you like to give that a try in a PR and add more tests?

[nosan]

I am not sure I can agree that stopping the registration of the bean's alias taskExecutor for the applicationTaskExecutor bean is not a breaking change. Many users might depend on the taskExecutor alias to fetch the Executor from a BeanFactory

A quick search on GitHub: https://github.com/search?q=getBean%28%22taskExecutor%22%29+OR+getBean%28%22taskExecutor%22%2C+Executor.class%29+OR+getBean%28%22taskExecutor%22%2C+SimpleAsyncTaskExecutor.class%29+OR+getBean%28%22taskExecutor%22%2C+ThreadPoolTaskExecutor.class%29+OR+getBean%28%22taskExecutor%22%2C+TaskExecutor.class%29+&type=code&p=1

Although not all results are relevant, it highlights that developers may be relying on this behavior. This doesn’t account for private repositories or other platforms like Bitbucket or GitLab.

Would you like to give that a try in a PR and add more tests?

I can prepare a PR to restore the 3.4.x behavior and include additional tests to address the concerns I’ve described above. Let me know if that works for you!

[wilkinsona]

We don't really consider bean names to be part of the public API. Generally speaking, and regardless of this change, we discourage retrieving an auto-configured bean by name. As @snicoll has said above, Boot's use of the taskExecutor name was really an implementation detail. That detail has now been replaced with the use of an AsyncConfigurer.

We'll document the change of name in the release notes in case it trips anyone up. That also gives us an opportunity to discourage name-based retrieval. Anyone who has code which cannot be changed and that relies on the taskExecutor bean name can use a bean factory post-processor to alias applicationTaskExecutor to taskExecutor.

[nosan]

Unfortunately, with these changes, the @Primary bean has no effect in force mode.

@Test
void enableAsyncUsesPrimaryExecutorWhenModeIsForceAndHasCustomTaskExecutor() {
	this.contextRunner
			.withPropertyValues("spring.task.execution.thread-name-prefix=auto-task-",
					"spring.task.execution.mode=force")
			.withBean("customTaskExecutor", Executor.class, () -> createCustomAsyncExecutor("custom-task-"),
					(bd) -> bd.setPrimary(true))
			.withUserConfiguration(AsyncConfiguration.class, TestBean.class)
			.run((context) -> {
				assertThat(context).hasSingleBean(AsyncConfigurer.class);
				assertThat(context.getBeansOfType(Executor.class)).hasSize(2);
				TestBean bean = context.getBean(TestBean.class);
				String text = bean.echo("something").get();
				assertThat(text).contains("custom-task-").contains("something"); //failed
			});
}

My intention was to preserve the existing logic and rely on the AsyncExecutionAspectSupport.getDefaultExecutor detection mechanism as it functioned before these changes.

In any case, I have shared all my concerns and received feedback, and it appears that you are either already aware of them or have an alternative approach in mind.

Thank you for your time.

[snicoll]

Unfortunately, with these changes, the @primary bean has no effect in force mode.

This is the advertized behaviour, see the note in the reference documentation. The idea of force is really to configure an executor even though one is present. This wasn't possible before and come with its own semantic.

If you use force, it doesn't matter what arrangement you have, we'll pick the auto-configured executor. Your only way out is to define your own AsyncConfigurer, and yes, you'll have then to implement all the other methods or you'll fallback on framework's default.

it appears that you are either already aware of them or have an alternative approach in mind.

Not all of them, so thanks for that. Based on your feedback we made some adjustments both in the code and in the release notes. From an upgrade perspective, only the taskExecutor name is a breaking change. Anything else is new stuff and I don't think it's fair, for instance, to expect your @Primary executor to be picked up in this new mode.