Skip to content

Reintroduce in-memory repositories #4050

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

Closed
marschall opened this issue Jan 28, 2022 · 9 comments
Closed

Reintroduce in-memory repositories #4050

marschall opened this issue Jan 28, 2022 · 9 comments
Labels
status: declined Features that we don't intend to implement or Bug reports that are invalid or missing enough details type: feature

Comments

@marschall
Copy link
Contributor

Expected Behavior

It should be possible to run Spring Batch integration tests without having to set up a database or if one is present without the tests committing to the database.

Current Behavior

The map based repositories were deprecated in v4 and removed in v5. However even they had the issue that they would commit to the database due to a transactional advice. This necessitates things like JobRepositoryTestUtils#removeJobExecutions.

Using an embedded database like H2, HSQL, etc does not solve the issue. The repositories still commit. In addition this only works when no other JDBC access is used. Otherwise multiple DataSource and DataSourceTransactionManager have to be juggled or the "real" database has to be used.

Context

I have implemented two custom JobRepository and JobExplorer implementations in marschall/spring-batch-inmemory.

  • Null implementations that do not save any data and therefore do not require any cleanup. This is ideal for integration tests where you don't want to have to clean up previous job executions. Additionally a NullDataSource is provided so that JobRepositoryTestUtils can be used.
  • In memory implementations that save all data in memory. These are intended as replacements for the deprecated the map based DAO implementations (MapJobInstanceDao, MapJobExecutionDao, MapStepExecutionDao and MapExecutionContextDao). These require clean up of previous job executions, the annotation @ClearJobRepository is provided to help with this. These avoid the performance problems of the map implementations by using copy constructors instead of Java serialization, this does not work if mutable objects are stored in contexts.

I would be willing to work on a PR.

See #4016, #3834

@marschall marschall added status: waiting-for-triage Issues that we did not analyse yet type: feature labels Jan 28, 2022
@fmbenhassine
Copy link
Contributor

Thank you for opening this issue and for sharing your GitHub repository!

It should be possible to run Spring Batch integration tests without having to set up a database or if one is present without the tests committing to the database.

I think this became possible after resolving #4070 (which was opened and resolved after this issue). Do you agree?

@fmbenhassine fmbenhassine added status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter and removed status: waiting-for-triage Issues that we did not analyse yet labels Mar 3, 2023
@marschall
Copy link
Contributor Author

Thank you for opening this issue and for sharing your GitHub repository!

It should be possible to run Spring Batch integration tests without having to set up a database or if one is present without the tests committing to the database.

I think this became possible after resolving #4070 (which was opened and resolved after this issue). Do you agree?

It depends.

Yes, JobRepositoryTestUtils is written against JobRepository but the only JobRepository supplied is SimpleJobRepository which relies on the *Daos which only have JDBC implementations. It is possible, but you have to supply your own implementation classes.

The commit can be avoided by avoiding in the same way by avoiding JobRepositoryFactoryBean. .

@fmbenhassine fmbenhassine removed the status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter label Apr 3, 2023
@fmbenhassine
Copy link
Contributor

Thank you for your feedback.

Expected Behavior

It should be possible to run Spring Batch integration tests without having to set up a database or if one is present without the tests committing to the database.

It is possible, but you have to supply your own implementation classes.

OK, since we agree that it is possible, I believe this issue is resolved, or more precisely, is no more an issue after #4070.

Now the fact that Spring Batch provides only a JDBC implementation of the various DAOs and that you need to supply your own implementation is not an issue in itself. What would be an issue is when Spring Batch does not provide a way for users to supply their custom implementations, but this is not the case (and your custom implementations are a good example for that).

The in-memory DAOs implementations were removed for the good reasons explained in #3780 and after careful thought by the team, and there is no plan to re-introduce them.

If you want, you can share your custom implementations in the Show and tell section of the project's discussions (we started using Github Discussions), and we would be happy to refer to them when appropriate.

Thank you.

@fmbenhassine fmbenhassine closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2023
@fmbenhassine fmbenhassine added the status: declined Features that we don't intend to implement or Bug reports that are invalid or missing enough details label Apr 3, 2023
@fmbenhassine
Copy link
Contributor

@marschall I just wanted to mention that we introduced a resourceless job repository implementation in 5.2.0-M2: #4679. Any feedback is welcome to improve things for the RC and then the GA in November.

@marschall
Copy link
Contributor Author

Thanks. I didn't think of the approach of keeping the last job instance and job execution. I think this will work for many people who don't need a job explorer. I think if a JobExplorer that accesses this JobRepository could be provided as well it may be even more useful.

@fmbenhassine
Copy link
Contributor

Thank you for your feedback! I was planning to add the job explorer in the GA.

@marschall
Copy link
Contributor Author

Something I missed is ResourcelessJobRepository not thread safe. Not sure this is an issue, a simple way to fix this was using a ThreadLocal.

@fmbenhassine
Copy link
Contributor

Yes, this is mentioned in the javadoc. In hindsight, I think it is better to make it thread safe for people running multiple jobs in the same JVMs with a thread-pool based job launcher. You are welcome to open a PR if you want.

@fmbenhassine
Copy link
Contributor

I was planning to add the job explorer in the GA.

Thinking about this in more details, I think it does not make sense to have a job explorer for a resourceless job repository. In fact, there is no resource to explore.. It is even not compatible with the current approach of using a single job instance/execution (encapsulated in the state) of the job repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined Features that we don't intend to implement or Bug reports that are invalid or missing enough details type: feature
Projects
None yet
Development

No branches or pull requests

2 participants