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

Application does not start anymore with only AbstractRoutingDataSource beans #18661

Closed
tokuhirom opened this issue Oct 19, 2019 · 7 comments
Assignees
Milestone

Comments

@tokuhirom
Copy link

@tokuhirom tokuhirom commented Oct 19, 2019

DataSourceHealthContributorAutoConfiguration loads all DataSources. And monitoring all data sources with an actuator.

It ignores AbstractRoutingDataSource... So it's a good implementation since the destination of AbstractRoutingDataSource may change.

@Bean
@ConditionalOnMissingBean(name = { "dbHealthIndicator", "dbHealthContributor" })
public HealthContributor dbHealthContributor(Map<String, DataSource> dataSources) {
return createContributor(filterDataSources(dataSources));
}
private Map<String, DataSource> filterDataSources(Map<String, DataSource> candidates) {
if (candidates == null) {
return null;
}
Map<String, DataSource> dataSources = new LinkedHashMap<>();
candidates.forEach((name, dataSource) -> {
if (!(dataSource instanceof AbstractRoutingDataSource)) {
dataSources.put(name, dataSource);
}
});
return dataSources;
}

But createContributor throws an exception when the argument is empty.

protected final C createContributor(Map<String, B> beans) {
Assert.notEmpty(beans, "Beans must not be empty");
if (beans.size() == 1) {
return createIndicator(beans.values().iterator().next());
}
return createComposite(beans);
}

As a result, if the module have only AbstractRoutingDataSources, boot application can't run.

@snicoll

This comment has been minimized.

Copy link
Member

@snicoll snicoll commented Oct 21, 2019

Thanks for the report. As far as I can see, we were checking the map isn't null in 2.1.x and the check is a bit more complete in 2.2.x. We need to figure out what to do if the map is empty.

@tokuhirom

This comment has been minimized.

Copy link
Author

@tokuhirom tokuhirom commented Oct 23, 2019

Here's a sample code to reproduce the issue.

https://github.com/tokuhirom/boot22-issue18661

@snicoll

This comment has been minimized.

Copy link
Member

@snicoll snicoll commented Oct 23, 2019

Thanks for the sample @tokuhirom. Here is the stacktrace:

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [org.springframework.boot.actuate.health.HealthContributor]: Factory method 'dbHealthContributor' threw exception; nested exception is java.lang.IllegalArgumentException: Beans must not be empty
        at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:185) ~[spring-beans-5.2.0.RELEASE.jar:5.2.0.RELEASE]
        at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:640) ~[spring-beans-5.2.0.RELEASE.jar:5.2.0.RELEASE]
        ... 105 common frames omitted
Caused by: java.lang.IllegalArgumentException: Beans must not be empty
        at org.springframework.util.Assert.notEmpty(Assert.java:549) ~[spring-core-5.2.0.RELEASE.jar:5.2.0.RELEASE]
        at org.springframework.boot.actuate.autoconfigure.health.AbstractCompositeHealthContributorConfiguration.createContributor(AbstractCompositeHealthContributorConfiguration.java:52) ~[spring-boot-actuator-autoconfigure-2.2.0.RELEASE.jar:2.2.0.RELEASE]
        at org.springframework.boot.actuate.autoconfigure.jdbc.DataSourceHealthContributorAutoConfiguration.dbHealthContributor(DataSourceHealthContributorAutoConfiguration.java:82) ~[spring-boot-actuator-autoconfigure-2.2.0.RELEASE.jar:2.2.0.RELEASE]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_202]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_202]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_202]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_202]
        at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.2.0.RELEASE.jar:5.2.0.RELEASE]
        ... 106 common frames omitted
@snicoll

This comment has been minimized.

Copy link
Member

@snicoll snicoll commented Oct 23, 2019

For the time being you can set management.health.db.enabled=false in your application configuration.

snicoll added a commit to snicoll/spring-boot that referenced this issue Oct 23, 2019
This commit extends the condition on `DataSource` to check that at least
one `DataSource` is not an `AbstractRoutingDataSource`. If the
application only defines such type, no candidates are available for the
health indicator and it should therefore not be registered.

Closes spring-projectsgh-18661
@snicoll

This comment has been minimized.

Copy link
Member

@snicoll snicoll commented Oct 23, 2019

If have a fix for a quite involved condition as I couldn't figure out how to bypass registration. I'd like a review from someone on the team.

@snicoll snicoll changed the title DataSourceHealthContributorAutoConfiguration with only AbstractRoutingDataSources Application does not start anymore with only AbstractRoutingDataSource beans Oct 23, 2019
@snicoll snicoll self-assigned this Oct 23, 2019
@snicoll

This comment has been minimized.

Copy link
Member

@snicoll snicoll commented Oct 23, 2019

We discussed this one at the team meeting and I realized I didn't ask or check what was happening with such setup in 2.1.x.

Here's the output for the sample above and 2.1.9.RELEASE:

{
  "status": "UP",
  "details": {
    "db": {
      "status": "UNKNOWN"
    },
    "diskSpace": {
      "status": "UP",
      "details": {
        "total": 499963170816,
        "free": 163580035072,
        "threshold": 10485760
      }
    }
  }
}

If we want this to be consistent, we could return a health indicator that returns UNKNOWN when no datasources are available. Perhaps we could use the opportunity to actually improve the situation?

@philwebb philwebb modified the milestones: 2.2.1, 2.2.x Oct 23, 2019
@philwebb philwebb assigned philwebb and unassigned snicoll Oct 23, 2019
@philwebb philwebb closed this in c5138c5 Oct 23, 2019
@philwebb philwebb modified the milestones: 2.2.x, 2.2.1 Oct 23, 2019
@philwebb

This comment has been minimized.

Copy link
Member

@philwebb philwebb commented Oct 23, 2019

Perhaps we could use the opportunity to actually improve the situation

I've update the logic so that routing datasource beans are now always shown. I've also added "routing" : true to the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.