Repository navigation
Considered new defaults for JPA repository bootstrap #16230
Description
Spring Boot 2.1 exposes a property spring.data.jpa.repositories.bootstrap-mode that can be set to default (default) deferred or lazy. I suggest do change the default value for property to be changed in the following scenarios:
- Change the general default to
deferred– it parallelizes the bootstrap of theEntityManagerFactoryand delays repository initialization to the end of theApplicationContextinitialization. I.e. there are performance benefits to the startup but the app is the same state as with the current default once it has been started. - Default to
lazyfor@SpringBootTest,@DataJpaTest– this leaves all but repositories injected into the tests initialized which gives a bit of a performance boost in both scenarios.
Activity
jnizet commentedon Mar 15, 2019
I wonder if going lazy for
SpringBootTestandDataJpaTestis a good idea.When creating a brand new project using initializr, a single test is typically created:
contextLoads.This test, as far as I understand, is meant to check that the context loads correctly, and thus that the application won't throw any exception once executed.
I understand that this is of course only one test, and that many others should be created, but still: with JPA repositories loaded lazily, this test will pass even though, for example, one of the JPA queries is invalid.
And, if DataJpaTest also uses lazy, if just one of the repositories happens not to be tested and is never injected anywhere, then an invalid JPA query in this repository will go unnoticed in tests.
Being faster is nice, but when it comes to tests, I'd rather have tests that are a tiny bit slower but that detect bugs.
What do you think?
wilkinsona commentedon Mar 16, 2019
I'm not in favour of any default that increases the risk of deploying something that's broken. Fast startup is all well and good, but I don't think it should be done at the expense of correctness. Other frameworks have made a different choice, and that may turn out to work well for their users, but I think it would be a mistake for us to follow suit purely to speed up startup time or test execution time.
Rather than making
@SpringBootTestor@DataJpaTestlazy by default, I'd prefer to add some of the discussion in this blog post to our reference documentation, along with some examples showing how lazy initialization can be enabled in tests (@SpringBootTest(properties="spring.main.lazy-initialization=true")should be all it takes). We could also consider an attribute on the@…Testannotations to make it even easier to opt in.ankurbhakta commentedon Mar 21, 2019
What are your thoughts of adding this optimizations as defaults in just spring boot devtools to improve the development experience?
odrotbohm commentedon Mar 21, 2019
Thanks for the feedback everyone. Very good points included. I guess I've been focussing too much on the how, whereas my actual concern was to make it very easy to make use of these configuration options.
I totally agree that we by no means should change the defaults in a way that currently working scenarios (like the general application bootstrap test that @jnizet described) should start to behave different in a less constraining way.
The proposal for making
deferredthe default still stands as the end result of the initialization is still equivalent to the current default, it just allows the asynchronousEntityManagerFactoryinitialization to take most effect possible.Regarding the lazy initialization, I agree that making it the default for DevTools is a good idea. For the test cases, my primary goal was to avoid spending time on initializing beans that are not required by a particular test. For
@Data…Test, there shouldn't be a need to initialize any repositories that are not injected into the test class. The ones injected will be fully initialized on first interaction. This should make practically no difference to the user experience as potential misconfigurations are still detected. Slightly later, but still. In fact, making other repositories lazy by default would result in those tests not fail due to bootstrap errors of unrelated repositories and thus provide a more precise picture of what's wrong exactly.As far as I can see this is currently needed to lazify repositories for such tests:
I wonder if this could be shortcutted to an attribute on the
@Data…Testannotations and default to lazy with the option for users to opt out of this again.wilkinsona commentedon Nov 22, 2019
We're going to:
deferredfor normal running and@SpringBootTestlazyfor@Data…Test@Data…Testthat uses theBootstrapModeannotation to change the defaultscottfrederick commentedon Jan 21, 2020
I'm not sure what the
...should mean in the@Data...Testannotations mentioned above, but JPA repositories are the only type of repositories that have abootstrapModeattribute on their respective@Enable...Repositoriesannotation in Spring Data, and the only type with abootstrap-modeproperty for Spring Boot auto-configuration.In addition, it is not currently possible to have more than one Spring Data repository configured with
BootstrapMode.DEFERRED(see https://jira.spring.io/browse/DATAJPA-1672). Changing the default toDEFERREDfor all types of repositories would have the potential to break any application that included more than one Spring DataRepositoryof any type.For these two reasons, we should change the default to
DEFERREDonly forJpaRepositoriesandLAZYonly for@DataJpaTest, leaving the default asDEFAULTfor all other types of Repositories.11 remaining items
ctmay4 commentedon May 26, 2020
I must have missed the change to the default, but this change affected both the projects I updated. Both intermittently would hang on start up and also when running tests. Changing the bootstrap mode back to
defaultseems to have fixed it (so far so good). I'm not usingorg.hibernate.search.FieldBridgeso I'm not sure my issue is the same as spring-projects/spring-framework#25111, but it was definitely a deadlock.wilkinsona commentedon May 26, 2020
@ctmay4 if you have the time, it would be great if you could capture and share a thread dump when things are deadlocked.
odrotbohm commentedon May 26, 2020
I've filed an issue for Spring Framework and @jhoeller seemed positive to be able to avoid a fix for the needed default constructor with the framework itself.
ctmay4 commentedon May 26, 2020
Here is the thread dump when startup is hanging.
odrotbohm commentedon May 26, 2020
Thanks, @ctmay4, that's very helpful. It looks like issue is caused by the Spring Data web integration being triggered and that trying to look up repositories during initialization and thus ending up waiting to be able to use the JPA meta-model currently in creation.
I think
DomainClassConverteris a decent place to delay that initialization until the first use of the converter.bukajsytlos commentedon May 26, 2020
In our case, we are using repositories during application startup
DATACMNS-1734 - Defer initialization of Repositories in DomainClassCo…
DATACMNS-1734 - Defer initialization of Repositories in DomainClassCo…
DATACMNS-1734 - Defer initialization of Repositories in DomainClassCo…
DATACMNS-1734 - Defer initialization of Repositories in DomainClassCo…
DATACMNS-1734 - Defer initialization of Repositories in DomainClassCo…
basven commentedon Jun 3, 2020
Not sure if related to this change since setting spring.data.jpa.repositories.bootstrap-mode=default
in my test application properties doesn't seem to help, but upgrading from 2.2.7 to 2.3.0 I see the following behavior:
For #1 we inject an entityManager in the aspect and noticed that the session is closed. This is only happening in the test environment; starting the war and calling some rest endpoint that calls in the end the repository does have the session 'open'.
For #2. The following scenario:
entityB = entityBrepository.findOne(id); <--- returns a proxied entity
entityA = new entityA(entityB);
entityArepository.save(entityA) <-- could not initialize proxy- no Session exception.
Tests are annotated as follows:
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
@ContextConfiguration
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
Update 1:
Mmm, might be a different issue altogether. We have a bean that listens to the ContextRefreshEvent and loads data if so configured. Here is when I see the above failures. But I noticed that when I use this bean at application startup (so outside the integration test environment) it has the same errors. Again this is all working in 2.2.7
Update 2:
So in 2.2.7 I receive 1 ContextRefreshEvent with source AnnotationConfigServletWebServerApplicationContext
With 2.3.0 I receive 2 of those events from the same source. If I discard the first event and only try to load data when I receive the event the second time, the above errors magically disappear.
So either I need to listen to a different event or that first event is emitted too soon?
Anyhow this discussion probably should move elsewhere, just no clue where.
wilkinsona commentedon Jun 5, 2020
@basven Could you please open a new issue so that we can investigate? A minimal sample that reproduces the problem would be much appreciated. If you are receiving two
ContextRefreshedEvents that's either a bug or there are two contexts involved. The latter would be true if you have configuredmanagement.server.portto run Actuator's HTTP endpoints on a separate port. This hasn't changed between 2.2 and 2.3 so may well not be the cause.hgarus commentedon Aug 18, 2020
I've run into this after updating to Spring Boot 2.3.3 from 2.2
So far I've only seen deadlocks in my tests.
Deadlocks seem to have disappeared after switiching my DataJpaTests to
BootstrapMode.DEFAULTand settingspring.data.jpa.repositories.bootstrap-modetodefault.A Thread Dump looks somewhat similar to ctmay4's:
JpaMetamodelMappingContext seems to be instantiated as a dependency of
org.springframework.data.auditing.AuditingHandler.snicoll commentedon Aug 18, 2020
@hgarus thank you for the report and sorry this is causing an issue for you. As indicated in the comment just above yours, could you please create a separate issue with a small sample that we can use to reproduce the issue?