ItemStreamSupport component name collision detection

[jpraet]

Alternative approach for #292.

Instead of trying to prevent ExecutionContext key collisions using
BeanNameAware, this PR throws an exception on update() when a duplicate
component name is detected.

[mminella]

This feels really late in the lifecycle to be throwing an error like this.

[jpraet]

Ideally it should be thrown when registering a stream, but when using a ClassifierCompositeItemWriter for example, the delegates usually won't get registered as a stream.

It could be checked in the open() but then we need some way to distinguish:

• a key already being present in the ExecutionContext because of a restart
• a key already being present in the ExecutionContext because of a non-unique name.

There is another problem with the current implementation as well: the case where you have for example a FlatFileItemWriter and a second MyCustomFlatFileItemWriter with the same name, it currently won't be detected as a name collision.

[pivotal-issuemaster]

@jpraet Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@jpraet Thank you for signing the Contributor License Agreement!

[jpraet]

We've had to deal with a production issue once again caused by this problem of multiple batch components of the same type persisting restart information in the execution context with colliding keys.

I feel like something should be done in the spring batch framework to prevent this common pitfall.

The issue specified in the reference docs, but it's just a little sidenote in chapter 6.13.1 Custom ItemReader Example:

It is also worth noting that the key used within the ExecutionContext should not be trivial. That is because the same ExecutionContext is used for all ItemStreams within a Step. In most cases, simply prepending the key with the class name should be enough to guarantee uniqueness. However, in the rare cases where two of the same type of ItemStream are used in the same step (which can happen if two files are need for output) then a more unique name will be needed. For this reason, many of the Spring Batch ItemReader and ItemWriter implementations have a setName() property that allows this key name to be overridden.

If nothing is done in the framework itself, I think it can be indicated more clearly as a warning in the docs. And should also be referred to from chapters 3.3 ExecutionContext and 6.4. ItemStream.

[mminella]

My concern still stands about breaking restartability between versions. I'm also not 100% sure that causing existing jobs to fail automatically because of this is also the ideal solution (if a step that uses a potentially colliding component is configured to not be restartable for example, the user wouldn't care). Since we already call this out in the documentation, I'm wondering if a simple warning log message when it occurs may be enough so that it's clear what is happening when it occurs.

[mminella]

To update on this, I'm having a conversation right now in BATCH-2575 about a potential breaking change between major versions related to this. If we do that, then it may make more sense to allow for this kind of change. Thoughts there are welcome!

[jpraet]

Yes, if restart compatibility is being broken between 3.x and 4.x, then I think it's best to merge #292 (which is a better solution to this problem than this PR, as it prevents the problem, instead of simply detecting it).

[fmbenhassine]

I think it's best to merge #292 (which is a better solution to this problem than this PR, as it prevents the problem, instead of simply detecting it)

@jpraet #292 has been closed. Is this PR still relevant?

[jpraet]

As far as I know the underlying problem (key collisions in execution context when using multiple batch components of the same type without configuring a name) still exists.

#292 resolve this problem (but was not accepted because it's not backwards compatible regarding job restarts).