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

Add support for Vibur DBCP connection pool to DataSourceBuilder #42903

Closed
simeonmalchev opened this issue Oct 28, 2024 · 9 comments
Closed

Add support for Vibur DBCP connection pool to DataSourceBuilder #42903

simeonmalchev opened this issue Oct 28, 2024 · 9 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@simeonmalchev
Copy link

Please add support / auto-configuration for Vibur DBCP connection pool (https://github.com/vibur/vibur-dbcp) in spring-boot.

Vibur DBCP is a concurrent, fast, and fully-featured JDBC connection pool, which has been around since 2013. The pool provides advanced performance monitoring capabilities, including slow SQL queries detection and logging, a non-starvation guarantee for application threads, statement caching, among other features.

Currently, spring-boot has built-in support for auto-configuration of the following jdbc connection pools in this order:

  1. DataSourceConfiguration.Hikari.class,
  2. DataSourceConfiguration.Tomcat.class
  3. DataSourceConfiguration.Dbcp2.class
  4. DataSourceConfiguration.OracleUcp.class

This issue is, in fact, to add Vibur DBCP to the above list. The Vibur DataSource class that needs to be instantiated is org.vibur.dbcp.ViburDBCPDataSource.class.

To the best of my understanding, adding support for Vibur will involve as a minimum changes in the DataSourceAutoConfiguration and DataSourceConfiguration classes from spring-boot-autoconfigure, and in the DataSourceBuilder class from spring-boot.

Note that Vibur requires its start() method to be called once the pool is configured. This method will validate the pool configuration. The pool also implements a close() method as part of its implementation of AutoClosebale.

If Vibur connection pool had to be instantiated from Java programming code, without the help of the spring.datasource.url, spring.datasource.username, spring.datasource.password, spring.datasource.driver-class-name properties, this could be done via something like:

    @Bean(initMethod = "start", destroyMethod = "close")
    @ConfigurationProperties(prefix = "spring.datasource.vibur")
    public DataSource dataSource() {
        return new ViburDBCPDataSource();
    }

The above snippet assumes that all Vibur config properties have a prefix of spring.datasource.vibur.
The disadvantage of such instantiation is that Vibur cannot utilize the mentioned earlier and standard for spring-boot 4 config properties with a prefix of spring.datasource. These 4 properties have to be repeated with a prefix of spring.datasource.vibur, where the url property needs be named jdbc-url as this is the name of the internal Vibur jdbc url property.

Disclaimer, I'm the author of Vibur DBCP. I'm happy to help with any questions which you may have about Vibur DBCP configuration or setup.

@simeonmalchev simeonmalchev changed the title Please add support / auto-configuration for Vibur DBCP connection pool (https://github.com/vibur/vibur-dbcp) in spring-boot. Please add support / auto-configuration for Vibur DBCP connection pool in spring-boot. Oct 28, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 28, 2024
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Oct 28, 2024
@snicoll snicoll changed the title Please add support / auto-configuration for Vibur DBCP connection pool in spring-boot. Consider adding support for Vibur DBCP connection pool Nov 3, 2024
@snicoll
Copy link
Member

snicoll commented Nov 3, 2024

Thanks for the suggestion.

As you may have seen, there is really two separate pieces to it:

  • Support of DataSourceBuilder so that programmatic use of it would transparently configure your datasource, including when using spring.datasource.url & co when specifying a datasource type.
  • Auto-configuration of a Viber DataSource if it's found on the classpath.

1 is a better place for your request, IMO. DataSourceBuilder has support for more pools such as C3P0, Oracle variants and the like. I've prototyped a quick hack that we can reuse if we want to pursue this.

@simeonmalchev
Copy link
Author

@snicoll, thanks a lot for looking into this ticket and thanks for prototyping the hack.

I'm thinking that if only the changes from the first piece that is in the DataSourceBuilder class from spring-boot are implemented and the changes from the second piece that is in the DataSourceAutoConfiguration and DataSourceConfiguration classes from spring-boot-autoconfigure are not implemented, this will be confusing for the Vibur-DBCP users as they won't be able to configure Vibur connection pool via the application.properties or application.yml file.

Currently, the other connection pools are configured via the 4 properties with prefix spring.datasource (e.g., spring.datasource.url) plus the pool specific properties which have different prefix for each pool. E.g., for Tomcat pool the specific properties prefix is spring.datasource.tomcat for Hikari pool the specific properties prefix is spring.datasource.hikari, etc. I could see that these pool specific properties are defined in the inner static classes that are part of the DataSourceConfiguration class.

For Vibur pool I would imagine that the pool specific properties prefix can be spring.datasource.vibur, however, this implies that the changes in the spring-boot-autoconfigure project are done. Also, it seems to me that a static class in the DataSourceConfiguration is the perfect place where the Vibur pool start method can be invoked via something like:

    @Configuration(proxyBeanMethods = false)
    @ConditionalOnClass(org.vibur.dbcp.ViburDBCPDataSource.class)
    @ConditionalOnMissingBean(DataSource.class)
    @ConditionalOnProperty(name = "spring.datasource.type", havingValue = "org.vibur.dbcp.ViburDBCPDataSource", matchIfMissing = true)
    static class Vibur {
        @Bean
        @ConditionalOnMissingBean(PropertiesJdbcConnectionDetails.class)
        static ViburJdbcConnectionDetailsBeanPostProcessor viburJdbcConnectionDetailsBeanPostProcessor(
                ObjectProvider<JdbcConnectionDetails> connectionDetailsProvider) {
            return new ViburJdbcConnectionDetailsBeanPostProcessor(connectionDetailsProvider);
        }

        @Bean(initMethod = "start", destroyMethod = "close")
        @ConfigurationProperties(prefix = "spring.datasource.vibur")
        org.vibur.dbcp.ViburDBCPDataSource dataSource(DataSourceProperties properties,
                                                      JdbcConnectionDetails connectionDetails) {
            org.vibur.dbcp.ViburDBCPDataSource dataSource = createDataSource(connectionDetails, org.vibur.dbcp.ViburDBCPDataSource.class,
                    properties.getClassLoader());
            if (StringUtils.hasText(properties.getName())) {
                dataSource.setName(properties.getName());
            }
            return dataSource;
        }
    }

If I'm misunderstanding some details about the connection pool's configuration in a spring-boot application, please point me out where. If, in fact, you're saying that is possible Vibur pool to be easily configured (including its specific properties) and used in a spring-boot application with only the changes that you proposed yesterday, can you please show an example how to do that?

@snicoll
Copy link
Member

snicoll commented Nov 5, 2024

@simeonmalchev please hold on until the team had a chance to review this.

If, in fact, you're saying that is possible Vibur pool to be easily configured (including its specific properties) and used in a spring-boot application with only the changes that you proposed yesterday, can you please show an example how to do that?

I never said that.

@simeonmalchev
Copy link
Author

No problem at all.

@philwebb
Copy link
Member

We discussed this today and we'd like to add support in the DataSourceBuilder but we're not keen to add any more auto-configured pools at this point.

I'll repurpose this issue and we'll merge @snicoll's prototype code later.

@philwebb philwebb changed the title Consider adding support for Vibur DBCP connection pool Add support for Vibur DBCP connection pool to DataSourceBuilder Nov 14, 2024
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Nov 14, 2024
@philwebb philwebb added this to the 3.5.x milestone Nov 14, 2024
@simeonmalchev
Copy link
Author

simeonmalchev commented Nov 16, 2024

@philwebb, thanks for the update.

I'm trying to figure out what will be the best way to post-configure the Vibur DBCP pool specific properties after the support for it is added in the DataSourceBuilder. Would this be via a BeanPostProcessor's postProcessAfterInitialization(..) method, or there is some better way how to set these properties?

@simeonmalchev
Copy link
Author

Hi @snicoll, I'm not sure if you'll get this message as this issue is now closed, but I just released a new version of Vibur-DBCP 26.0 yesterday. Can you please update the Vibur version in your commit above, or I should try myself to make a PR for updating it?

@snicoll
Copy link
Member

snicoll commented Jan 10, 2025

Neither. The commit has been pushed, we can't change it. We don't need a PR as we have a semi-automated process that does it. Your case is however special has we don't generally upgrade to new majors in a feature release automatically so I'll keep an eye on it.

@simeonmalchev
Copy link
Author

@snicoll, thanks! I'm apologizing for the major version change. Vibur doesn't follow a strict semver as x.y.z, it just does x.y and usually each new release changes the x. It just started like that long time ago...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants