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

Proper default value for skipLimit #4661

Closed
samuelstein opened this issue Sep 12, 2024 · 3 comments
Closed

Proper default value for skipLimit #4661

samuelstein opened this issue Sep 12, 2024 · 3 comments

Comments

@samuelstein
Copy link

samuelstein commented Sep 12, 2024

Recently I stumbled over the default value of the skipLimit in the class FaultTolerantStepBuilder. It is set by default to 0.
If you don't use a SkipPolicy and configure it instead with skip(Exception.class) it is very easy to forget setting the additional property skipLimit.
Could you provide a more suitable default value?
My suggestion would be Integer.MAX_VALUE.
What do you think?

@samuelstein samuelstein added the status: waiting-for-triage Issues that we did not analyse yet label Sep 12, 2024
@fmbenhassine
Copy link
Contributor

fmbenhassine commented Sep 16, 2024

That's a valid point, thank you for raising it!

My suggestion would be Integer.MAX_VALUE

This means that by default we would theoretically allow the entire dataset to be skipped, which in turn means there is something fundamentally wrong with the input (bad data format, unexpected input, etc). As a user, I would prefer my job to fail fast in the first dozen or hundred skips rather than waiting until the end just to discover that my job was skipping the entire data set. Do you see my point?

I guess 10 or 100 is a reasonable default. Wdyt?

I will plan the change in the next minor release.

@samuelstein
Copy link
Author

samuelstein commented Sep 17, 2024

Hi @fmbenhassine,
this makes abolutely sense to me. I would prefer at least 10. Then you have a good balance between fail-fast and misconfiguration.

@Ian3110
Copy link
Contributor

Ian3110 commented Sep 22, 2024

I have opened a pull request that addresses this issue: #4668

@fmbenhassine fmbenhassine modified the milestones: 5.2.0-M2, 5.2.0-RC1 Oct 10, 2024
@fmbenhassine fmbenhassine modified the milestones: 5.2.0-RC1, 5.2.0 Oct 23, 2024
fmbenhassine added a commit that referenced this issue Nov 20, 2024
Before this commit, an assertion was enforcing
that when a skip limit is provided, then at least
one skippable exception is defined.

Since the default value of skip limit was changed
to 10 in fd45d32, that assertion is now replaced
with a log message at debug level.

Related to #4661
FBibonne pushed a commit to FBibonne/spring-batch that referenced this issue Feb 2, 2025
FBibonne pushed a commit to FBibonne/spring-batch that referenced this issue Feb 2, 2025
Before this commit, an assertion was enforcing
that when a skip limit is provided, then at least
one skippable exception is defined.

Since the default value of skip limit was changed
to 10 in fd45d32, that assertion is now replaced
with a log message at debug level.

Related to spring-projects#4661

Signed-off-by: Fabrice Bibonne <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants