fix: adds idle timeout and minimum idle configs (#184)

* fix: adds idle timeout and minimum idle configs * fix: protected props * fix: changelog * fix: test protected config
supertokens · Feb 9, 2024 · 5e27c9e · 5e27c9e
1 parent abe5312
commit 5e27c9e
Show file tree

Hide file tree

Showing 8 changed files with 233 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Fixes the issue where passwords were inadvertently logged in the logs.
 - Adds tests to check connection pool behaviour.
+- Adds `postgresql_idle_connection_timeout` and `postgresql_minimum_idle_connections` configs to control active connections to the database.
 
 ## [5.0.6] - 2023-12-05
 

diff --git a/config.yaml b/config.yaml
@@ -67,3 +67,11 @@ postgresql_config_version: 0
 # (DIFFERENT_ACROSS_TENANTS | OPTIONAL | Default: "thirdparty_users") string value. Specify the name of the table
 # that will store the thirdparty recipe users.
 # postgresql_thirdparty_users_table_name
+
+# (DIFFERENT_ACROSS_TENANTS | OPTIONAL | Default: 60000) long value. Timeout in milliseconds for the idle connections
+# to be closed.
+# postgresql_idle_connection_timeout:
+
+# (DIFFERENT_ACROSS_TENANTS | OPTIONAL | Default: 1) integer value. Minimum number of idle connections to be kept
+# active.
+# postgresql_minimum_idle_connections:
diff --git a/devConfig.yaml b/devConfig.yaml
@@ -69,3 +69,11 @@ postgresql_password: "root"
 # (DIFFERENT_ACROSS_TENANTS | OPTIONAL | Default: "thirdparty_users") string value. Specify the name of the table
 # that will store the thirdparty recipe users.
 # postgresql_thirdparty_users_table_name
+
+# (DIFFERENT_ACROSS_TENANTS | OPTIONAL | Default: 60000) long value. Timeout in milliseconds for the idle connections
+# to be closed.
+# postgresql_idle_connection_timeout:
+
+# (DIFFERENT_ACROSS_TENANTS | OPTIONAL | Default: 1) integer value. Minimum number of idle connections to be kept
+# active.
+# postgresql_minimum_idle_connections:
diff --git a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java
@@ -81,6 +81,8 @@ private synchronized void initialiseHikariDataSource() throws SQLException {
         }
         config.setMaximumPoolSize(userConfig.getConnectionPoolSize());
         config.setConnectionTimeout(5000);
+        config.setIdleTimeout(userConfig.getIdleConnectionTimeout());
+        config.setMinimumIdle(userConfig.getMinimumIdleConnections());
         config.addDataSourceProperty("cachePrepStmts", "true");
         config.addDataSourceProperty("prepStmtCacheSize", "250");
         config.addDataSourceProperty("prepStmtCacheSqlLimit", "2048");

diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java
@@ -117,7 +117,8 @@ public class Start
     // SaaS. If the core is not running in SuperTokens SaaS, this array has no effect.
     private static String[] PROTECTED_DB_CONFIG = new String[]{"postgresql_connection_pool_size",
             "postgresql_connection_uri", "postgresql_host", "postgresql_port", "postgresql_user", "postgresql_password",
-            "postgresql_database_name", "postgresql_table_schema"};
+            "postgresql_database_name", "postgresql_table_schema", "postgresql_idle_connection_timeout",
+            "postgresql_minimum_idle_connections"};
     private static final Object appenderLock = new Object();
     public static boolean silent = false;
     private ResourceDistributor resourceDistributor = new ResourceDistributor();

diff --git a/src/main/java/io/supertokens/storage/postgresql/config/PostgreSQLConfig.java b/src/main/java/io/supertokens/storage/postgresql/config/PostgreSQLConfig.java
@@ -112,6 +112,14 @@ public class PostgreSQLConfig {
     @ConnectionPoolProperty
     private String postgresql_connection_scheme = "postgresql";
 
+    @JsonProperty
+    @ConnectionPoolProperty
+    private long postgresql_idle_connection_timeout = 60000;
+
+    @JsonProperty
+    @ConnectionPoolProperty
+    private int postgresql_minimum_idle_connections = 1;
+
     @IgnoreForAnnotationCheck
     boolean isValidAndNormalised = false;
 
@@ -234,6 +242,14 @@ public String getThirdPartyUsersTable() {
         return postgresql_thirdparty_users_table_name;
     }
 
+    public long getIdleConnectionTimeout() {
+        return postgresql_idle_connection_timeout;
+    }
+
+    public int getMinimumIdleConnections() {
+        return postgresql_minimum_idle_connections;
+    }
+
     public String getThirdPartyUserToTenantTable() {
         return addSchemaAndPrefixToTableName("thirdparty_user_to_tenant");
     }
@@ -340,6 +356,17 @@ public void validateAndNormalise() throws InvalidConfigException {
                     "'postgresql_connection_pool_size' in the config.yaml file must be > 0");
         }
 
+        if (postgresql_minimum_idle_connections <= 0) {
+            throw new InvalidConfigException(
+                    "'postgresql_minimum_idle_connections' must be a positive value");
+        }
+
+        if (postgresql_minimum_idle_connections > postgresql_connection_pool_size) {
+            throw new InvalidConfigException(
+                    "'postgresql_minimum_idle_connections' must be less than or equal to "
+                            + "'postgresql_connection_pool_size'");
+        }
+
         // Normalisation
         if (postgresql_connection_uri != null) {
             { // postgresql_connection_attributes

diff --git a/src/test/java/io/supertokens/storage/postgresql/test/DbConnectionPoolTest.java b/src/test/java/io/supertokens/storage/postgresql/test/DbConnectionPoolTest.java
@@ -16,32 +16,34 @@
 
 package io.supertokens.storage.postgresql.test;
 
+import static org.junit.Assert.*;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+
+import io.supertokens.pluginInterface.multitenancy.*;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+
 import com.google.gson.JsonObject;
+
 import io.supertokens.ProcessState;
 import io.supertokens.emailpassword.exceptions.EmailChangeNotAllowedException;
 import io.supertokens.featureflag.EE_FEATURES;
 import io.supertokens.featureflag.FeatureFlagTestContent;
 import io.supertokens.multitenancy.Multitenancy;
 import io.supertokens.multitenancy.exception.BadPermissionException;
 import io.supertokens.pluginInterface.exceptions.StorageQueryException;
-import io.supertokens.pluginInterface.multitenancy.*;
 import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException;
 import io.supertokens.storage.postgresql.Start;
 import io.supertokens.storageLayer.StorageLayer;
 import io.supertokens.thirdparty.ThirdParty;
-import org.junit.AfterClass;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TestRule;
-
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicLong;
-
-import static org.junit.Assert.*;
 
 public class DbConnectionPoolTest {
     @Rule
@@ -64,6 +66,7 @@ public void testActiveConnectionsWithTenants() throws Exception {
         TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false);
         FeatureFlagTestContent.getInstance(process.getProcess())
                 .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY});
+        Utils.setValueInConfig("postgresql_minimum_idle_connections", "10");
         process.startProcess();
         assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));
 
@@ -87,6 +90,7 @@ public void testActiveConnectionsWithTenants() throws Exception {
 
         // change connection pool size
         config.addProperty("postgresql_connection_pool_size", 20);
+        config.addProperty("postgresql_minimum_idle_connections", 20);
 
         Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
                 new TenantIdentifier(null, null, "t1"),
@@ -119,6 +123,7 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
             FeatureFlagTestContent.getInstance(process.getProcess())
                     .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY});
             process.startProcess();
+            Utils.setValueInConfig("postgresql_minimum_idle_connections", "10");
             assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));
 
             Start start = (Start) StorageLayer.getBaseStorage(process.getProcess());
@@ -127,6 +132,7 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
             JsonObject config = new JsonObject();
             start.modifyConfigToAddANewUserPoolForTesting(config, 1);
             config.addProperty("postgresql_connection_pool_size", 300);
+            config.addProperty("postgresql_minimum_idle_connections", 300);
             AtomicLong firstErrorTime = new AtomicLong(-1);
             AtomicLong successAfterErrorTime = new AtomicLong(-1);
             AtomicInteger errorCount = new AtomicInteger(0);
@@ -190,6 +196,7 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
 
             // change connection pool size
             config.addProperty("postgresql_connection_pool_size", 200);
+            config.addProperty("postgresql_minimum_idle_connections", 200);
 
             Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
                     new TenantIdentifier(null, null, "t1"),
@@ -232,4 +239,165 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
 
         fail(); // tried 5 times
     }
+
+
+    @Test
+    public void testMinimumIdleConnections() throws Exception {
+        String[] args = {"../"};
+        {
+            TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false);
+            FeatureFlagTestContent.getInstance(process.getProcess())
+                    .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY});
+            Utils.setValueInConfig("postgresql_connection_pool_size", "20");
+            Utils.setValueInConfig("postgresql_minimum_idle_connections", "10");
+            Utils.setValueInConfig("postgresql_idle_connection_timeout", "30000");
+            process.startProcess();
+            assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));
+
+            Thread.sleep(65000); // let the idle connections time out
+
+            Start start = (Start) StorageLayer.getBaseStorage(process.getProcess());
+            assertEquals(10, start.getDbActivityCount("supertokens"));
+
+            process.kill();
+            assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
+        }
+    }
+
+    @Test
+    public void testMinimumIdleConnectionForTenants() throws Exception {
+        String[] args = {"../"};
+
+        TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false);
+        FeatureFlagTestContent.getInstance(process.getProcess())
+                .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY});
+        Utils.setValueInConfig("postgresql_minimum_idle_connections", "10");
+        process.startProcess();
+        assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));
+
+        Start start = (Start) StorageLayer.getBaseStorage(process.getProcess());
+        assertEquals(10, start.getDbActivityCount("supertokens"));
+
+        JsonObject config = new JsonObject();
+        start.modifyConfigToAddANewUserPoolForTesting(config, 1);
+
+        Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
+                new TenantIdentifier(null, null, "t1"),
+                new EmailPasswordConfig(true),
+                new ThirdPartyConfig(true, null),
+                new PasswordlessConfig(true),
+                config
+        ), false);
+
+        Thread.sleep(1000); // let the new tenant be ready
+
+        assertEquals(10, start.getDbActivityCount("st1"));
+
+        // change connection pool size
+        config.addProperty("postgresql_connection_pool_size", 20);
+        config.addProperty("postgresql_minimum_idle_connections", 5);
+
+        Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
+                new TenantIdentifier(null, null, "t1"),
+                new EmailPasswordConfig(true),
+                new ThirdPartyConfig(true, null),
+                new PasswordlessConfig(true),
+                config
+        ), false);
+
+        Thread.sleep(2000); // let the new tenant be ready
+
+        assertEquals(5, start.getDbActivityCount("st1"));
+
+        // delete tenant
+        Multitenancy.deleteTenant(new TenantIdentifier(null, null, "t1"), process.getProcess());
+        Thread.sleep(2000); // let the tenant be deleted
+
+        assertEquals(0, start.getDbActivityCount("st1"));
+
+        process.kill();
+        assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
+    }
+
+    @Test
+    public void testIdleConnectionTimeout() throws Exception {
+        String[] args = {"../"};
+
+        TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false);
+        FeatureFlagTestContent.getInstance(process.getProcess())
+                .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY});
+        Utils.setValueInConfig("postgresql_minimum_idle_connections", "10");
+        process.startProcess();
+        assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));
+
+        Start start = (Start) StorageLayer.getBaseStorage(process.getProcess());
+        assertEquals(10, start.getDbActivityCount("supertokens"));
+
+        JsonObject config = new JsonObject();
+        start.modifyConfigToAddANewUserPoolForTesting(config, 1);
+        config.addProperty("postgresql_connection_pool_size", 300);
+        config.addProperty("postgresql_minimum_idle_connections", 5);
+        config.addProperty("postgresql_idle_connection_timeout", 30000);
+
+        AtomicLong errorCount = new AtomicLong(0);
+
+        Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
+                new TenantIdentifier(null, null, "t1"),
+                new EmailPasswordConfig(true),
+                new ThirdPartyConfig(true, null),
+                new PasswordlessConfig(true),
+                config
+        ), false);
+
+        Thread.sleep(3000); // let the new tenant be ready
+
+        assertTrue(10 >= start.getDbActivityCount("st1"));
+
+        ExecutorService es = Executors.newFixedThreadPool(150);
+
+        for (int i = 0; i < 10000; i++) {
+            int finalI = i;
+            es.execute(() -> {
+                try {
+                    TenantIdentifier t1 = new TenantIdentifier(null, null, "t1");
+                    TenantIdentifierWithStorage t1WithStorage = t1.withStorage(StorageLayer.getStorage(t1, process.getProcess()));
+                    ThirdParty.signInUp(t1WithStorage, process.getProcess(), "google", "googleid"+ finalI, "user" +
+                            finalI + "@example.com");
+
+                } catch (StorageQueryException e) {
+                    errorCount.incrementAndGet();
+                    throw new RuntimeException(e);
+                } catch (EmailChangeNotAllowedException e) {
+                    errorCount.incrementAndGet();
+                    throw new RuntimeException(e);
+                } catch (TenantOrAppNotFoundException e) {
+                    errorCount.incrementAndGet();
+                    throw new RuntimeException(e);
+                } catch (BadPermissionException e) {
+                    errorCount.incrementAndGet();
+                    throw new RuntimeException(e);
+                }
+            });
+        }
+
+        es.shutdown();
+        es.awaitTermination(2, TimeUnit.MINUTES);
+
+        assertTrue(5 < start.getDbActivityCount("st1"));
+
+        assertEquals(0, errorCount.get());
+
+        Thread.sleep(65000); // let the idle connections time out
+
+        assertEquals(5, start.getDbActivityCount("st1"));
+
+        // delete tenant
+        Multitenancy.deleteTenant(new TenantIdentifier(null, null, "t1"), process.getProcess());
+        Thread.sleep(3000); // let the tenant be deleted
+
+        assertEquals(0, start.getDbActivityCount("st1"));
+
+        process.kill();
+        assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
+    }
 }
diff --git a/src/test/java/io/supertokens/storage/postgresql/test/SuperTokensSaaSSecretTest.java b/src/test/java/io/supertokens/storage/postgresql/test/SuperTokensSaaSSecretTest.java
@@ -45,12 +45,12 @@ public class SuperTokensSaaSSecretTest {
 
     private final String[] PROTECTED_DB_CONFIG = new String[]{"postgresql_connection_pool_size",
             "postgresql_connection_uri", "postgresql_host", "postgresql_port", "postgresql_user",
-            "postgresql_password",
-            "postgresql_database_name", "postgresql_table_schema"};
+            "postgresql_password", "postgresql_database_name", "postgresql_table_schema",
+            "postgresql_minimum_idle_connections", "postgresql_idle_connection_timeout"};
 
     private final Object[] PROTECTED_DB_CONFIG_VALUES = new Object[]{11,
             "postgresql://root:root@localhost:5432/supertokens?currentSchema=myschema", "localhost", 5432, "root",
-            "root", "supertokens", "myschema"};
+            "root", "supertokens", "myschema", 5, 120000};
 
     @Rule
     public TestRule watchman = Utils.getOnFailure();