Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add StepExecution parameter to StoppableTasklet.stop() #4703

Open
andrianov17 opened this issue Nov 10, 2024 · 4 comments · May be fixed by #4715
Open

Add StepExecution parameter to StoppableTasklet.stop() #4703

andrianov17 opened this issue Nov 10, 2024 · 4 comments · May be fixed by #4715
Labels
in: core status: feedback-provided Issues for which the feedback requested from the reporter was provided type: feature
Milestone

Comments

@andrianov17
Copy link

Currently, StoppableTasklet.stop() doesn't take any parameters. So, from within tasklet it is impossible to distinguish which exactly step execution is requested to stop out of multiple job executions running. This is important when some external request needs to be executed from StoppableTasklet.stop().

Looking into SimpleJobOperator.stop(long executionId) implementation, it looks very straightforward to implement this request - just pass stepExecution to the tasklet stop() method:

@Override
	public boolean stop(long executionId) throws NoSuchJobExecutionException, JobExecutionNotRunningException {

		JobExecution jobExecution = findExecutionById(executionId);
		// Indicate the execution should be stopped by setting it's status to
		// 'STOPPING'. It is assumed that
		// the step implementation will check this status at chunk boundaries.
		BatchStatus status = jobExecution.getStatus();
		if (!(status == BatchStatus.STARTED || status == BatchStatus.STARTING)) {
			throw new JobExecutionNotRunningException(
					"JobExecution must be running so that it can be stopped: " + jobExecution);
		}
		jobExecution.setStatus(BatchStatus.STOPPING);
		jobRepository.update(jobExecution);

		try {
			Job job = jobRegistry.getJob(jobExecution.getJobInstance().getJobName());
			if (job instanceof StepLocator) {// can only process as StepLocator is the
												// only way to get the step object
				// get the current stepExecution
				for (StepExecution stepExecution : jobExecution.getStepExecutions()) {
					if (stepExecution.getStatus().isRunning()) {
						try {
							// have the step execution that's running -> need to 'stop' it
							Step step = ((StepLocator) job).getStep(stepExecution.getStepName());
							if (step instanceof TaskletStep) {
								Tasklet tasklet = ((TaskletStep) step).getTasklet();
								if (tasklet instanceof StoppableTasklet) {
									StepSynchronizationManager.register(stepExecution);
									((StoppableTasklet) tasklet).stop(**stepExecution**);
									StepSynchronizationManager.release();
								}
							}
						}

Thank you in advance!

@andrianov17 andrianov17 added the status: waiting-for-triage Issues that we did not analyse yet label Nov 10, 2024
@fmbenhassine fmbenhassine added type: feature in: core and removed status: waiting-for-triage Issues that we did not analyse yet labels Nov 11, 2024
@fmbenhassine fmbenhassine added this to the 6.0.0 milestone Nov 11, 2024
@HyunSangHan
Copy link

Can I work on this issue?

@HyunSangHan
Copy link

I have submitted a PR!: #4715

@fmbenhassine
Copy link
Contributor

Thank you for opening this feature request. I am not sure I fully understand the need for this, so would like to make sure the requirement is clear enough to me.

From the Javadocs of StoppableTasklet, the idea is that stopping a given job execution would stop all stoppable tasklets, which makes sense to me since we want to stop the overall job execution (ie it would not make sense to be selective to which tasklet to stop). For example, if I have three instances of the same stoppable tasklet or even different stoppable tasklets running in parallel, then if I request to stop the job execution, I am expecting all tasklets to get the stop signal, and not a selected subset. Is my understanding correct?

I am also not sure, from an API design point of view, about the added value of having the step execution in the stop method:

import org.springframework.batch.core.StepContribution;
import org.springframework.batch.core.StepExecution;
import org.springframework.batch.core.scope.context.ChunkContext;
import org.springframework.batch.core.step.tasklet.StoppableTasklet;
import org.springframework.batch.repeat.RepeatStatus;

public class MyStoppableTasklet implements StoppableTasklet {

    @Override
    public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
        // do some work
        return RepeatStatus.FINISHED;
    }

    @Override
    public void stop(StepExecution stepExecution) {
        // We have just received a signal to stop the execution at this point,
        // what is the added value of stepExecution here?
    }
}

Again, I might be missing a use case, so please do not hesitate to elaborate on this.

@fmbenhassine fmbenhassine added the status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter label Feb 25, 2025
@andrianov17
Copy link
Author

What you've explained would be fully sufficient if there were no external async components involved in the tasklet execution.

We use tasklet to wrap JMS call to external system, and the tasklet waits for response completion. For simplicity, let's say one job has just one tasklet.

Now, let's say there 3 job instances running, each of which is waiting for JMS completion. They have send JMS messages with correlation IDs: A, B, C. Now we want to stop only second job (waiting for correlation ID B). If we call stop() we don't have enough context to send cancellation message with correlation ID B to cancel it in external system. On the other hand, having step execution ID (as part of stepExecution parameter), we are able to find respective correlation ID in our map created before the message was sent for processing.

@fmbenhassine fmbenhassine added status: feedback-provided Issues for which the feedback requested from the reporter was provided and removed status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter labels Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: feedback-provided Issues for which the feedback requested from the reporter was provided type: feature
Projects
None yet
3 participants