spring-cloud / spring-cloud-consul

Spring Cloud Consul
http://cloud.spring.io/spring-cloud-consul/
Apache License 2.0
813 stars 541 forks source link

Make catalogWatchTaskScheduler/configWatchTaskScheduler more sophisticated #723

Open v-ladynev opened 3 years ago

v-ladynev commented 3 years ago

This issue related to #146. A fix provided there is not suitable. For example:

  1. The consul is disabled for a local environment and my own configuration for TaskScheduler with 2 threads works well.
  2. The consul is enabled for PROD configuration and there is a single scheduling thread as a result.

To fix this a spring-vault approach with TaskSchedulerWrapper can be used https://github.com/spring-projects/spring-vault/blob/main/spring-vault-core/src/main/java/org/springframework/vault/config/AbstractVaultConfiguration.java#L324

The advise in the documentation is not very good:

The watch uses a Spring TaskScheduler to schedule the call to consul. By default it is a ThreadPoolTaskScheduler with a poolSize of 1. To change the TaskScheduler, create a bean of type TaskScheduler named with the ConsulConfigAutoConfiguration.CONFIG_WATCH_TASK_SCHEDULER_NAME constant

  1. It doesn't work. It needs to enable bean overriding.
  2. Why I have to add to my application configuration a bean with a wired name, just to have multiple threads in the TaskScheduler?

The simplest workaround is not to use a bean with the same name, but @Primary on the application configuration.

    @Primary
    @Bean(name = "app-scheduler-pool")
    public TaskScheduler appSchedulerPool() {
        ThreadPoolTaskScheduler threadPoolTaskScheduler = new ThreadPoolTaskScheduler();
        threadPoolTaskScheduler.setPoolSize(THREADS_COUNT);
        return threadPoolTaskScheduler;
    }
spencergibb commented 3 years ago

this was created before spring boot changed the default of bean overriding. The docs could be updated to reflect primary as an option. I don't understand how a wrapper will help.

spring-cloud-issues commented 3 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

v-ladynev commented 3 years ago

@spencergibb Maybe a wrapper will not help. I just wanted to say that spring-vault has a separate TaskScheduler that doesn't influence my application scheduling logic at all. spring-cloud-consul does it.

spencergibb commented 3 years ago

After chatting with the team in the spring cloud kubernetes issue, we're doing a wrapper as a bean, we should do the same here.