-
Notifications
You must be signed in to change notification settings - Fork 41.3k
Closed
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 theEntityManagerFactory
and delays repository initialization to the end of theApplicationContext
initialization. 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
lazy
for@SpringBootTest
,@DataJpaTest
– this leaves all but repositories injected into the tests initialized which gives a bit of a performance boost in both scenarios.
kamkie, desmethans, sergey-morenets, derTobsch, ismakc and 1 more
Metadata
Metadata
Assignees
Labels
type: enhancementA general enhancementA general enhancement
Type
Projects
Relationships
Development
Select code repository
Activity
jnizet commentedon Mar 15, 2019
I wonder if going lazy for
SpringBootTest
andDataJpaTest
is 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
@SpringBootTest
or@DataJpaTest
lazy 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@…Test
annotations 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
deferred
the default still stands as the end result of the initialization is still equivalent to the current default, it just allows the asynchronousEntityManagerFactory
initialization 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…Test
annotations and default to lazy with the option for users to opt out of this again.wilkinsona commentedon Nov 22, 2019
We're going to:
deferred
for normal running and@SpringBootTest
lazy
for@Data…Test
@Data…Test
that uses theBootstrapMode
annotation to change the defaultscottfrederick commentedon Jan 21, 2020
I'm not sure what the
...
should mean in the@Data...Test
annotations mentioned above, but JPA repositories are the only type of repositories that have abootstrapMode
attribute on their respective@Enable...Repositories
annotation in Spring Data, and the only type with abootstrap-mode
property 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 toDEFERRED
for all types of repositories would have the potential to break any application that included more than one Spring DataRepository
of any type.For these two reasons, we should change the default to
DEFERRED
only forJpaRepositories
andLAZY
only for@DataJpaTest
, leaving the default asDEFAULT
for all other types of Repositories.18 remaining items
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
DomainClassConverter
is 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
ContextRefreshedEvent
s that's either a bug or there are two contexts involved. The latter would be true if you have configuredmanagement.server.port
to 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.DEFAULT
and settingspring.data.jpa.repositories.bootstrap-mode
todefault
.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?