spring-attic / spring-boot-r2dbc

Experimental Spring Boot support for R2DBC
82 stars 13 forks source link

ConnectionFactoryAutoConfiguration runs before a ConnectionPool bean is configured #80

Closed eduardo-bueno-ifood closed 4 years ago

eduardo-bueno-ifood commented 4 years ago

I was trying to set up the auto-configured metrics for r2dbc and noticed that ConnectionPoolMetricsAutoConfiguration was not being touched in Spring initialization. Turns out that the annotation @ConditionalOnBean({ ConnectionPool.class, MeterRegistry.class }) does not wait for a ConnectionPool to be available and thus this configuration never runs. I have gone around this by providing a ConnectionPool bean in a configuration class and marking it with @AutoConfigureBefore(ConnectionPoolMetricsAutoConfiguration::class).

snicoll commented 4 years ago

@eduardo-bueno-ifood I am not entirely sure I got that. AutoConfigureBefore only works on auto-configuration so it wouldn't have any impact on a user configuration.

What auto-configuration doesn't wait for a ConnectionPool to be available exactly?

eduardo-bueno-ifood commented 4 years ago

It is ConnectionPoolMetricsAutoConfiguration - sorry I have messed up classes' names. Edited my post.

eduardo-bueno-ifood commented 4 years ago

It seems that @AutoConfigureBefore is not impacting indeed; just providing a io.r2dbc.pool.ConnectionPool is enough. This is not the case for the auto-configured one though. If I don't provide my own ConnectionPool then the metrics auto-configuration never runs.

snicoll commented 4 years ago

thanks for the feedback. Which version of this project are you using? There is @AutoConfigureAfter on R2dbcAutoConfiguration which should definitely make sure that a ConnectionPool is auto-configured first, see https://github.com/spring-projects-experimental/spring-boot-r2dbc/commit/b9c62359cc6589505b4368186db9a59dd95b75e1#diff-dda599f23a475e85b2c4c7e1389a3bd0

If you're not using 0.1.0.M3 yet, please upgrade. If you do, we'll need a small sample (github repo url or a zip we can download) to reproduce the problem you've described.

eduardo-bueno-ifood commented 4 years ago

I'm using 0.1.0.M3 indeed. While testing further I noticed that the issue is not reproduced when switching from PostgreSQL to H2, so maybe the issue lies within the ConnectionPool setup for PostgreSQL.

snicoll commented 4 years ago

@eduardo-bueno-ifood if you could share a small sample that would be helpful please.

eduardo-bueno-ifood commented 4 years ago

I have set up this repository as an example: https://github.com/eduardo-bueno-ifood/r2dbc-actuator-demo It is identical to the r2dbc actuator example except it uses PostgreSQL instead of H2. I have created a free elephantSQL instance for this example so I don't care about the credentials.

snicoll commented 4 years ago

Thanks for the sample and I can reproduce with both M3 and the latest state of this repository and the current Spring Boot integration.

Something is going on when you configure pooling in the URL rather than using the properties. When you do that, the standard ConnectionFactory mechanism is used and r2dbc's driver lookup mechanism ultimately creates a ConnectionPool but the condition does not seem to be able to catch that.

Your current configuration is wrong by the way: if you configure pool then pooling options must be set in the url as well and any .pool options are ignored. I've changed the URL and removed :pool to let this auto-configuration creates a pool explicitly (since that's the default behaviour when r2dbc-pool is on the classpath anyway) and things are working as expected.

To be clear, rather than:

spring.r2dbc.url=r2dbc:pool:postgresql://raja.db.elephantsql.com:5432/dtexgfnf
spring.r2dbc.username=...
spring.r2dbc.password=...

spring.r2dbc.pool.initial-size=3

This, that actually takes that initial-size property into account:

spring.r2dbc.url=r2dbc:postgresql://raja.db.elephantsql.com:5432/dtexgfnf
spring.r2dbc.username=...
spring.r2dbc.password=...

spring.r2dbc.pool.initial-size=3

This experimental project has moved to Spring Boot proper (you can try 2.3.0.BUILD-SNAPSHOT) so I've created an issue over there to address this issue: https://github.com/spring-projects/spring-boot/issues/20349

I'd remove :pool in the meantime (and definitely if you are configuring pooling options outside of the URL).

Thank you for reporting the issue and following-up so quickly on my questions.