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

Allow Spring Data JPA's bootstrap mode to be configured via the environment #13833

Closed
dsyer opened this issue Jul 20, 2018 · 12 comments
Closed
Labels
type: enhancement A general enhancement
Milestone

Comments

@dsyer
Copy link
Member

dsyer commented Jul 20, 2018

Setting RepositoryFactoryBeanSupport.lazyInit to true (in a BeanPostProcessor) speeds up the Petclinic startup by about 10%. Maybe a good thing to expose as a @ConfigurationProperties option in Spring Boot?

N.B. if you want to try it, remember to add @Lazy to the injection points of all the repositories as well (all controllers).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 20, 2018
@dsyer dsyer added the theme: performance Issues related to general performance label Jul 20, 2018
@wilkinsona
Copy link
Member

What do you think @olivergierke?

@dsyer
Copy link
Member Author

dsyer commented Jul 20, 2018

It's actually more than 10% for the Petclinic. Startup time is down from 5000ms to 3600ms on my laptop.

@dsyer
Copy link
Member Author

dsyer commented Jul 23, 2018

See also https://jira.spring.io/browse/DATAJPA-1380 (if fixed it would make it easier to prevent eager initialization of the EntityManagerFactory in a slightly different code path).

@odrotbohm
Copy link
Member

odrotbohm commented Jul 25, 2018

We've investigated that option quite a while ago in the context of Spring Framework adding support for parallel EntityManager instantiation but concluded to not do anything for then as what Dave mentions in his original post is crucial and likely to get wrong for developers: all components using a repository instance would have to mark the injection point as lazy and not make any use of the repository inside the constructor as otherwise that will cause the entire optimization to break.

IIRC, I briefly spoke with Jürgen about framework means to enforce the "lazinization" of a component based on a flag on its bean definition, so that our configuration code could add that based on a configuration switch, but we didn't pursue that any further.

Another issue that played into our decision is that the repository bootstrap already performs query validation for manually defined queries, interacts with the JPA metamodel etc. I.e. a lazification of repositories in general will lead to the app to bootstrap into an unverified state, which in a bad case will just cause an initialization error to occur on first request. Even worse: with JPA we have the problem, that we need to trigger exception-causing code (to detect whether a query method is backed by a named query), which – according to the spec – will have to trigger a transaction rollback. Even if the actual request can be performed properly. That leads to the weird effect of a first request hitting a particular repository to result in an error, but all follow up ones to succeed.

To avoid that situation is actually one of the reasons we explicitly introduced the ability to activate eager initialization in our CDI extension as that is lazy by default, which makes that weird scenario the default case.

That said, I am still willing to reconsider if we got better support from the framework side to consistently enable lazy injection for a certain set of bean definitions. I can also picture e.g. the Spring Boot dev tools to enable that to be introduced switch, as I'd argue we can assume the validation to be executed in integration tests and during "normal" application startup.

Does that help?

/cc @jhoeller

@dsyer
Copy link
Member Author

dsyer commented Jul 25, 2018

Devtools is probably a good scenario to push this feature a bit harder, and of course integration tests. If the option becomes available we could make it the default in devtools (for instance) but optional otherwise. That way issues to do with query validation etc. as described above would have to be opted into.

@odrotbohm
Copy link
Member

A quick heads up on the progress so far:

We have tickets and prototypical implementations for DATACMNS-1368 and DATAJPA-1397 (consume via a manual Spring Data JPA dependency declaration using 2.1.0.DATAJPA-1397-SNAPSHOT). The latter exposes a new attribute bootstrapMode in @EnableJpaRepositories with non-default values of LAZY and DEFERRED, consumed via RepositoryConfigurationSource.getBootstrapMode().

LAZY makes injection points for repositories lazy by default. This will allow downstream components to be initialized without blocking on an asynchonously created EntityManagerFactory. It also means that repositories will still be uninitialized – and thus: unvalidated – on application bootstrap finish. DEFERRED is basically the same but will trigger repository initialization on ContextRefreshedEvent so that problems in repository setup are still detected before the application signals its okay.

Based on that I'd love to see an application property to trigger both the deferred mode and the asynchronous EntityManagerFactory initialization so that users can easily flip this on in their configuration. Devtools could then switch this on by default.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Aug 13, 2018
@wilkinsona
Copy link
Member

Here is a prototype of some changes to Boot that allow the bootstrap mode to be configured using spring.data.jpa.repositories.bootstrap-mode. Things seem to work as far as I can tell. The modified Data JPA sample results in the repository bean definitions being marked for lazy init.

I guess we also need a property for asynchronous entity manager initialisation. @olivergierke can you educate me on how that's enabled please?

@odrotbohm
Copy link
Member

odrotbohm commented Aug 14, 2018

You basically register a TaskExecutor on the LocalContainerEntityManagerFactoryBean like shown here.

I wonder if we should use a slightly broader property name as ideally, the developer would only have to tweak one to trigger both aspects as they only really make sense in combination with each other and are likely (read: will have to) to be set together. Or maybe we can have another property that triggers both then, but I guess then we'd have to resolve them being set in conflicting ways. Also, – although not strictly required – that mode works for all repositories, i.e. using something that's not tied to the JPA namespace might be worth considering as well.

In general, I think the asynchronous EMF bootstrap in combination with the deferred mode is a decent choice even for production scenarios as it combines the faster startup but still keeps the validation in the right spot. I'd love to see that combination to be easy to activate to eventually even make it the default. Devtools could then even go further and set the mode to lazy to prevent the repository initialization entirely.

@wilkinsona
Copy link
Member

We can't provide a single broader-named property that customises the bootstrap mode and enables asynchronous initialisation of the EMF without only allowing one bootstrap mode to be specified, unless that property's values are the three supported bootstrap modes and we infer from that whether or not the task executor should be configured on the EMF. At that point, I think we might as well stick with a bootstrap-mode property and infer from its value if the EMF should be configured with a task executor.

Also, – although not strictly required – that mode works for all repositories, i.e. using something that's not tied to the JPA namespace might be worth considering as well.

That's interesting. If it were a Data-wide property (rather than being tied to a specific store) I'd be less sure about inferring the EMF's task executor configuration from the property's value. Given that we already have individual properties for auto-enabling of each repository type (the spring.data.*.repositories.enabled properties) I'm leaning towards having individual bootstrap mode properties too. I can imagine a scenario where users may want to use LAZY for one store and DEFERRED for another and individual properties would allow them to do so without having to use the @Enable…Repositories annotations.

@odrotbohm
Copy link
Member

I'm fine with the property name as is. My core concern was that I'd like to see a single property to switch on all necessary bits and pieces. If you can derive the EMF bootstrap mode from the repository one, awesome.

@odrotbohm
Copy link
Member

odrotbohm commented Aug 16, 2018

The feature branches in Spring Data Commons and JPA are merged into master now. I also prepared a more complete example to showcase the effect of the different modes.

Feel free to ping me once the Boot support is ready, so that I can update it to throw out the additional (then obsolete) custom configuration.

@wilkinsona wilkinsona changed the title Make it easy to configure Spring Data repositories as lazy Allow Spring Data JPA's bootstrap mode to be configured via the environment Aug 16, 2018
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged theme: performance Issues related to general performance labels Aug 16, 2018
@wilkinsona wilkinsona modified the milestones: 2.1.0.M2, Backlog Aug 16, 2018
@wilkinsona
Copy link
Member

I've retitled this issue to reflect the fact that we're focussing on Data JPA for now. We could add similar properties for the other repository types if that goes well.

wilkinsona added a commit to wilkinsona/spring-boot that referenced this issue Aug 16, 2018
In Spring Data Lovelace, repositories' bootstrap mode can be
configured via @EnableJpaRepositories. This commit adds support for
configuring the mode via the environment rather than having to use
the annotation. Additionally, when deferred or lazy bootstrapping is
being used, the LocalContainerEntityManagerFactoryBean is configured
to use a bootstrap executor. This allows JPA's initialization to be
performed on a separate thread, allowing the rest of application
context initialization to proceed in parallel.

Closes spring-projectsgh-13833
@wilkinsona wilkinsona modified the milestones: Backlog, 2.1.0.M2 Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants