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

Missing note about not scoping Step beans with Job scope #3900

Closed
kzander91 opened this issue Apr 30, 2021 · 3 comments
Closed

Missing note about not scoping Step beans with Job scope #3900

kzander91 opened this issue Apr 30, 2021 · 3 comments
Labels
for: backport-to-5.1.x Issues that will be back-ported to the 5.1.x line has: minimal-example Bug reports that provide a minimal complete reproducible example in: documentation type: bug
Milestone

Comments

@kzander91
Copy link

Bug description
I have a Job that contains a Step that is annotated with @JobScope (to be able to inject job parameters).
Using JobOperator.stop() from another thread to stop that job fails with the following exception:

org.springframework.beans.factory.support.ScopeNotActiveException: Error creating bean with name 'scopedTarget.step': Scope 'job' is not active for the current thread; consider defining a scoped proxy for this bean if you intend to refer to it from a singleton; nested exception is java.lang.IllegalStateException: No context holder available for job scope
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:383) ~[spring-beans-5.3.6.jar:5.3.6]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.6.jar:5.3.6]
	at org.springframework.aop.target.SimpleBeanTargetSource.getTarget(SimpleBeanTargetSource.java:35) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:195) ~[spring-aop-5.3.6.jar:5.3.6]
	at com.sun.proxy.$Proxy48.getName(Unknown Source) ~[na:na]
	at org.springframework.batch.core.job.SimpleJob.getStep(SimpleJob.java:109) ~[spring-batch-core-4.3.2.jar:4.3.2]
	at org.springframework.batch.core.launch.support.SimpleJobOperator.stop(SimpleJobOperator.java:402) ~[spring-batch-core-4.3.2.jar:4.3.2]
	at org.springframework.batch.core.launch.support.SimpleJobOperator$$FastClassBySpringCGLIB$$44ee6049.invoke(<generated>) ~[spring-batch-core-4.3.2.jar:4.3.2]
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218) ~[spring-core-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:779) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:750) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:123) ~[spring-tx-5.3.6.jar:5.3.6]
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:388) ~[spring-tx-5.3.6.jar:5.3.6]
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119) ~[spring-tx-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:750) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:692) ~[spring-aop-5.3.6.jar:5.3.6]
	at org.springframework.batch.core.launch.support.SimpleJobOperator$$EnhancerBySpringCGLIB$$2759e0b7.stop(<generated>) ~[spring-batch-core-4.3.2.jar:4.3.2]
	at com.example.demo.DemoApplication.lambda$runner$0(DemoApplication.java:54) ~[classes/:na]
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:819) ~[spring-boot-2.4.5.jar:2.4.5]
	at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:803) ~[spring-boot-2.4.5.jar:2.4.5]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:346) ~[spring-boot-2.4.5.jar:2.4.5]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1340) ~[spring-boot-2.4.5.jar:2.4.5]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1329) ~[spring-boot-2.4.5.jar:2.4.5]
	at com.example.demo.DemoApplication.main(DemoApplication.java:91) ~[classes/:na]
Caused by: java.lang.IllegalStateException: No context holder available for job scope
	at org.springframework.batch.core.scope.JobScope.getContext(JobScope.java:159) ~[spring-batch-core-4.3.2.jar:4.3.2]
	at org.springframework.batch.core.scope.JobScope.get(JobScope.java:92) ~[spring-batch-core-4.3.2.jar:4.3.2]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:371) ~[spring-beans-5.3.6.jar:5.3.6]
	... 25 common frames omitted

Environment
Spring Batch: 4.3.2

Steps to reproduce

  1. Configure job with job-scoped steps.
  2. Run job asynchronously (to get hold of the pending JobExecution).
  3. Use JobOperator.stop() to stop that job.

Expected behavior
Stopping the job works, regardless of the scope of its steps.

Minimal Complete Reproducible example
Sample project:
demo.zip

The project contains a job-scoped step. A CommandLineRunner creates a JobLauncher that launches jobs asynchronously.
Then, the main thread waits two seconds before it calls JobOperator.stop().

  1. Unzip.
  2. Run ./mvnw spring-boot:run
  3. Two seconds after the job was started, JobOperator.stop() is called and fails with the exception above.
@kzander91 kzander91 added status: waiting-for-triage Issues that we did not analyse yet type: bug labels Apr 30, 2021
@fmbenhassine fmbenhassine added has: minimal-example Bug reports that provide a minimal complete reproducible example in: core labels Apr 30, 2021
@fmbenhassine
Copy link
Contributor

Thank you for opening this issue and for providing a minimal example.

The problem here is the scoped step. A step should not be scoped (with step scope or job scope). what should be scoped instead is the component of the step (tasklet, reader, writer, etc). In your case, it is the item processor that should be scoped to use late-binding, not the step itself. Here is an example of how your sample should be changed:

    @Bean
    Job job() {
        return jobs.get("job") //
                .start(step()) //
                .build();
    }

    @Bean
    Step step() {
        return steps.get("step") //
                .<String, String>chunk(1) //
                .reader(() -> String.valueOf(Math.random())) //
                .processor(itemProcessor(null)) //
                .writer(new ListItemWriter<>()) //
                .build();
    }

    @Bean
    @StepScope
    public ItemProcessor<String, String> itemProcessor(@Value("#{jobParameters['param']}") String param ) {
        return item -> {
            log.info("Processing item: '{}' Param: '{}'", item, param);
            TimeUnit.SECONDS.sleep(1L);
            return item;
        };
    }

With those changes, the sample works as expected.

There is a mention about not scoping Step beans in the documentation here, but it only mentions step-scope while it should include job scope as well.

I will change this into a documentation issue and update the docs accordingly.

@fmbenhassine fmbenhassine added in: documentation and removed in: core status: waiting-for-triage Issues that we did not analyse yet labels Jun 14, 2023
@fmbenhassine fmbenhassine changed the title JobOperator.stop() fails if Job contains @JobScoped steps Missing note about not scoping Step beans with Job scope Jun 14, 2023
@marbon87
Copy link
Contributor

marbon87 commented Jan 29, 2024

Hi @fmbenhassine ,

if steps should have scope step or job, how can i dynamically set the chunkSize from a job parameter?
In https://stackoverflow.com/a/67365622/4073333 your advice was to use @JobScope for the step, but that is not righy anymore, is it?

edit:

Is this a good solution?

    @Bean
    @StepScope
    public SimpleCompletionPolicy chunkCompletionPolicy(@Value("#{jobParameters['chunkSize']}") int chunkSize) {
        return new SimpleCompletionPolicy(chunkSize);
    }


    @Bean
    public Step step(PlatformTransactionManager transactionManager,
                     SimpleCompletionPolicy simpleCompletionPolicy) {
        return steps
                .get("exampleStep")
                .chunk(simpleCompletionPolicy, transactionManager)
                .reader(reader1())
                .processor(processor())
                .writer(dbResultWriter())
                .build();
    }

@fmbenhassine fmbenhassine added this to the 5.2.0 milestone Oct 23, 2024
@fmbenhassine fmbenhassine modified the milestones: 5.2.0, 5.2.1 Nov 20, 2024
@fmbenhassine
Copy link
Contributor

There is a mention about not scoping Step beans in the documentation here, but it only mentions step-scope while it should include job scope as well. I will change this into a documentation issue and update the docs accordingly.

Done: 7481cf8

@marbon87 I don't know how I missed your message, apologies. Indeed, I talked about job-scoped steps in that answer on stackoverflow, but it seems like I should have elaborated more on the available options and, more importantly what is recommended and what is not. I updated that answer with more details: https://stackoverflow.com/a/67365622/5019386

Is this a good solution?

Yes, just like the item reader or writer, the completion policy is the component to scope instead of the step itself. The docs were updated accordingly. I mentioned the fact that a step should not be scoped on SO, but I will iterate it here: while technically possible, a step should not be scoped. The reason is simple: scoping a step does not make sense, in fact it means: do not create that step definition until this step is started at runtime, but to start that step at runtime it needs to be defined upfront.. While one might think that the the step-scoped is for everything except steps, which should be job-scoped, the job scope was actually introduced for the sole reason to support JSR 352, which is was removed in v5. This is really a discussion for another thread, but if someone is using the job scope, then she/he is doing something wrong.

fmbenhassine added a commit that referenced this issue Dec 18, 2024
@fmbenhassine fmbenhassine added the for: backport-to-5.1.x Issues that will be back-ported to the 5.1.x line label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: backport-to-5.1.x Issues that will be back-ported to the 5.1.x line has: minimal-example Bug reports that provide a minimal complete reproducible example in: documentation type: bug
Projects
None yet
Development

No branches or pull requests

3 participants