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

R2DBC ConnectionFactory bean .close() method should be called/subscribed when available #26991

Closed
elefeint opened this issue May 27, 2021 · 4 comments
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Milestone

Comments

@elefeint
Copy link

When Spring Boot autoconfiguration instantiates io.r2dbc.spi.ConnectionFactory bean implementing io.r2dbc.spi.Closeable, the framework should have a custom destruction method invoke Closeable.close() and subscribe to the returned Publisher.

If a user application creates its own ConnectionFactory, it would be the user's responsibility to then close it. Although it might be nice to have a generic pre-destruction facility that looks for all R2DBC Closeable beans and closes them cleanly. The regular Spring bean destruction mechanism won't work here, since Spring Framework would call .close() but not subscribe to it, making it a no-op.

cc: @mp911de

@philwebb philwebb transferred this issue from spring-projects/spring-boot May 27, 2021
@philwebb
Copy link
Member

I've transferred this issue to the Spring Framework team to see if they can add a support class to spring-r2dbc to handle this. We could then auto-configure the class in Spring Boot. I wonder if some kind of post-processor is needed to deal with this?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 27, 2021
@jhoeller jhoeller self-assigned this May 28, 2021
@jhoeller jhoeller added in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 28, 2021
@jhoeller jhoeller added this to the 5.3.8 milestone May 28, 2021
@jhoeller
Copy link
Contributor

I wonder whether we should revise the core bean destroy method handling to detect a reactive return type and automatically subscribe to it... @rstoyanchev does that sound feasible? Calling a close method with a publisher returned and then ignoring it seems entirely wrong.

@mp911de
Copy link
Member

mp911de commented May 28, 2021

When discussing asynchronous/reactive close methods, we should also consider what happens when a close method returns a future. This works today already, but there's no synchronization/error handling.

Subscribing to a close Publisher would make a lot of sense to be at least consistent with close methods returning a future.

@rstoyanchev
Copy link
Contributor

I agree that a close method with an async or reactive return type shouldn't be ignored and that it should be consistent with the handling of similar with a Future. The ReactiveAdapterRegistry might come handy for common handling of all known or registered types.

The question of synchronization is interesting too, whether to await completion (possibly with some configurable timeout) but also how do it, i.e. sequentially, in parallel, or some dependency sequence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants