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

fix: adds idle timeout and minimum idle configs #184

Merged
merged 5 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
8 changes: 8 additions & 0 deletions devConfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,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));

Expand All @@ -87,6 +88,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"),
Expand Down Expand Up @@ -117,6 +119,7 @@ public void testDownTimeWhenChangingConnectionPoolSize() 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));

Expand All @@ -126,6 +129,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);
Expand Down Expand Up @@ -157,7 +161,7 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
successAfterErrorTime.set(System.currentTimeMillis());
}
} catch (StorageQueryException e) {
if (e.getMessage().contains("Connection is closed")) {
if (e.getMessage().contains("Connection is closed") || e.getMessage().contains("has been closed")) {
if (firstErrorTime.get() == -1) {
firstErrorTime.set(System.currentTimeMillis());
}
Expand All @@ -180,6 +184,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"),
Expand Down Expand Up @@ -210,4 +215,164 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
process.kill();
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
rishabhpoddar marked this conversation as resolved.
Show resolved Hide resolved
}

@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));
}
}
Loading