Batch hangs when throttle limit exceeds pool size + queue capacity in multithreaded step

[farnetto]

If throttleLimit exceeds the ThreadPoolExecutor pool size plus the jobQueue capacity (and using the default RejectedExecutionHandler), then the batch hangs when the job queue capacity in the executor is exceeded. Here is an example:

@Configuration
@EnableBatchProcessing
public class GH3947a {
	
	private static final ConcurrentLinkedQueue<Integer> ITEMS = new ConcurrentLinkedQueue<Integer>(IntStream.rangeClosed(1, 100).boxed().collect(Collectors.toList())); 

    @Bean
    ThreadPoolTaskExecutor taskExecutor() {
        ThreadPoolTaskExecutor taskExecutor = new ThreadPoolTaskExecutor();
        taskExecutor.setCorePoolSize(10);
        taskExecutor.setMaxPoolSize(10);
        taskExecutor.setQueueCapacity(10);
        taskExecutor.setRejectedExecutionHandler(new ThreadPoolExecutor.AbortPolicy() {
        	public void rejectedExecution(Runnable r, ThreadPoolExecutor e) {
        		System.out.println("execution rejected");
        		super.rejectedExecution(r, e);
        	}
        });
        taskExecutor.setDaemon(true);
        return taskExecutor;
    }

    @Bean
    public Job job(JobBuilderFactory jobs, StepBuilderFactory steps) {
        return jobs.get("job")
                .start(steps.get("step")
                		.<Integer,Integer>chunk(1)
                		.reader(() -> ITEMS.poll())
                		.processor(processor())
                		.writer(items -> System.out.println(items))
                        .taskExecutor(taskExecutor())
                        .throttleLimit(21)
                        .build())
                .build();
    }
    
    private ItemProcessor<Integer, Integer> processor() {
		return new ItemProcessor<Integer, Integer>() {
			@Override
			public Integer process(Integer item) throws Exception {
				TimeUnit.SECONDS.sleep(1);
				return item;
			}
		};
	}

	public static void main(String[] args) throws Exception {
        ApplicationContext context = new AnnotationConfigApplicationContext(GH3947a.class);
        JobLauncher jobLauncher = context.getBean(JobLauncher.class);
        Job job = context.getBean(Job.class);
        jobLauncher.run(job, new JobParameters());
    }
}

The RejectedExecutionException is added to the deferred list in RepeatTemplate.doHandle and the batch hangs waiting for results.

[fmbenhassine]

Thank you for opening this issue and for providing an example. I confirm that the execution hangs in this case. However, it is clearly mentioned in the Javadoc of TaskExecutorRepeatTemplate.html#setThrottleLimit that the thread pool size should be larger than the throttle limit:

when used with a thread pooled TaskExecutor the thread pool might prevent the throttle limit actually being reached
(so make the core pool size larger than the throttle limit if possible).

If you make the core pool size larger than the throttle limit (by setting the throttle limit to 9 for example), your example works as expected. So the current implementation works as designed/documented. Hence, I'm closing this issue.

That said, and without re-iterating all arguments discussed in #3947, we are aware that this throttle-limit parameter is causing several confusions, issues and performance degradation (#1516), that's why it will be deprecated and removed (#2218).

[farnetto]

Yes, the best thing to do when someone points out a deadlock in your code is to close the bug quickly and pretend the problem doesn't exist.

[mminella]

@farnetto I want to take a minute and thank you for the work you put into these issues. The samples and digging into the code have helped have a constructive dialog on this and the related issue (#3947). However I do want to point out that we try to maintain an inclusive environment where all members of the community are comfortable being involved without fear of personal attacks, trolling or insulting/derogatory comments, or other unprofessional conduct (per our code of conduct: https://github.com/spring-projects/.github/blob/main/CODE_OF_CONDUCT.md). This last comment here and the last one on #3947 are moving away from constructive technical conversation to things that we want to avoid.

We are all working on this together and we appreciate your help in moving the frameworks forward for the entire community.

[farnetto]

I've pointed out a couple of minor problems (for #3948 I even submitted a PR), and a couple of severe problems with your code (i.e. hanging in two different situations that are not so unlikely), that have apparently been there undetected since version 1 (2008), and all I've gotten is condescension ("I see you're confused, let me explain what you're doing wrong"), and now I'm getting a lecture. No, thanks. If you had just said, "you've got a point, the javadoc is not quite right, we'll fix it," this wouldn't have escalated. See for reference the start of #3947. Now please just consider my points seriously and make a plausible plan for improving things in v5. Removing the throttle limit won't be easy, if that's it. I've looked at the code.