From b7c06d6c609a8f9dd904746b33e7cb87bd6f2d58 Mon Sep 17 00:00:00 2001 From: sneha122 Date: Sat, 1 Feb 2025 00:36:19 +0530 Subject: [PATCH] fix: added configurable connection pool support for mssql (#38818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description With current implementation, MSSQL is configured with HikariCP to have maximum pool size of 10. But this means that if multiple concurrent users are accessing multiple queries of the same datasource at the same time, some of the queries might time out due to max pool size of 10. We have encountered this issue in https://theappsmith.slack.com/archives/C0341RERY4R/p1736150680663059. Where 40-50 concurrent users are simultaneously making 20-30 queries to MSSQL database. This PR makes this connection pool size configurable from admin settings for MSSQL as well. Fixes #https://github.com/appsmithorg/appsmith-ee/issues/5928 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > ๐ŸŸข ๐ŸŸข ๐ŸŸข All cypress tests have passed! ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ > Workflow run: > Commit: ebda9364200b4df19c86b8ec4e68a95d6180f10c > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Thu, 23 Jan 2025 11:18:35 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **New Features** - Enhanced connection pool configuration for MSSQL plugin - Added support for dynamically setting maximum connection pool size - **Improvements** - Increased flexibility in database connection management - Implemented configurable connection pool settings - **Tests** - Added mock configuration for testing connection pool settings Co-authored-by: โ€œsneha122โ€ <โ€œsneha@appsmith.comโ€> --- .../com/external/plugins/MssqlPlugin.java | 27 ++++++++++++++----- .../plugins/MssqlTestDBContainerManager.java | 12 ++++++++- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java b/app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java index 8521d9dc4e..5f47a5cf9d 100644 --- a/app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java +++ b/app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java @@ -1,5 +1,6 @@ package com.external.plugins; +import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfig; import com.appsmith.external.constants.DataType; import com.appsmith.external.datatypes.AppsmithType; import com.appsmith.external.dtos.ExecuteActionDTO; @@ -114,6 +115,12 @@ public class MssqlPlugin extends BasePlugin { private static final int PREPARED_STATEMENT_INDEX = 0; + private final ConnectionPoolConfig connectionPoolConfig; + + public MssqlPluginExecutor(ConnectionPoolConfig connectionPoolConfig) { + this.connectionPoolConfig = connectionPoolConfig; + } + /** * Instead of using the default executeParametrized provided by pluginExecutor, this implementation affords an opportunity * to use PreparedStatement (if configured) which requires the variable substitution, etc. to happen in a particular format @@ -356,9 +363,13 @@ public class MssqlPlugin extends BasePlugin { @Override public Mono datasourceCreate(DatasourceConfiguration datasourceConfiguration) { log.debug(Thread.currentThread().getName() + ": datasourceCreate() called for MSSQL plugin."); - return Mono.fromCallable(() -> { - log.debug(Thread.currentThread().getName() + ": Connecting to SQL Server db"); - return createConnectionPool(datasourceConfiguration); + return connectionPoolConfig + .getMaxConnectionPoolSize() + .flatMap(maxPoolSize -> { + return Mono.fromCallable(() -> { + log.debug(Thread.currentThread().getName() + ": Connecting to SQL Server db"); + return createConnectionPool(datasourceConfiguration, maxPoolSize); + }); }) .subscribeOn(scheduler); } @@ -561,8 +572,8 @@ public class MssqlPlugin extends BasePlugin { * @param datasourceConfiguration * @return connection pool */ - private static HikariDataSource createConnectionPool(DatasourceConfiguration datasourceConfiguration) - throws AppsmithPluginException { + private static HikariDataSource createConnectionPool( + DatasourceConfiguration datasourceConfiguration, Integer maxPoolSize) throws AppsmithPluginException { DBAuth authentication = null; StringBuilder urlBuilder = null; @@ -572,7 +583,11 @@ public class MssqlPlugin extends BasePlugin { hikariConfig = new HikariConfig(); hikariConfig.setDriverClassName(JDBC_DRIVER); hikariConfig.setMinimumIdle(MINIMUM_POOL_SIZE); - hikariConfig.setMaximumPoolSize(MAXIMUM_POOL_SIZE); + + // Use maxPoolSize from config if available, otherwise use default + int maximumPoolSize = maxPoolSize != null ? maxPoolSize : MAXIMUM_POOL_SIZE; + hikariConfig.setMaximumPoolSize(maximumPoolSize); + // Configuring leak detection threshold for 60 seconds. Any connection which hasn't been released in 60 seconds // should get tracked (may be falsely for long running queries) as leaked connection hikariConfig.setLeakDetectionThreshold(LEAK_DETECTION_TIME_MS); diff --git a/app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlTestDBContainerManager.java b/app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlTestDBContainerManager.java index 26d833547f..a3deb96730 100644 --- a/app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlTestDBContainerManager.java +++ b/app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlTestDBContainerManager.java @@ -1,5 +1,6 @@ package com.external.plugins; +import com.appsmith.external.configurations.connectionpool.ConnectionPoolConfig; import com.appsmith.external.models.DBAuth; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Endpoint; @@ -8,6 +9,7 @@ import com.external.plugins.utils.MssqlDatasourceUtils; import com.zaxxer.hikari.HikariDataSource; import org.testcontainers.containers.MSSQLServerContainer; import org.testcontainers.utility.DockerImageName; +import reactor.core.publisher.Mono; import java.sql.SQLException; import java.sql.Statement; @@ -18,7 +20,15 @@ import static com.external.plugins.utils.MssqlExecuteUtils.closeConnectionPostEx public class MssqlTestDBContainerManager { - static MssqlPlugin.MssqlPluginExecutor mssqlPluginExecutor = new MssqlPlugin.MssqlPluginExecutor(); + private static class MockConnectionPoolConfig implements ConnectionPoolConfig { + @Override + public Mono getMaxConnectionPoolSize() { + return Mono.just(5); + } + } + + static MssqlPlugin.MssqlPluginExecutor mssqlPluginExecutor = + new MssqlPlugin.MssqlPluginExecutor(new MockConnectionPoolConfig()); public static MssqlDatasourceUtils mssqlDatasourceUtils = new MssqlDatasourceUtils();