diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index 16a403682..a876daee5 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1512,11 +1512,21 @@ public final class RegistryConfig { return CONFIG_SETTINGS.get().hibernate.hikariConnectionTimeout; } + /** Returns the minimum idle connections for HikariCP. */ + public static String getHibernateHikariMinimumIdle() { + return CONFIG_SETTINGS.get().hibernate.hikariMinimumIdle; + } + /** Returns the maximum pool size for HikariCP. */ public static String getHibernateHikariMaximumPoolSize() { return CONFIG_SETTINGS.get().hibernate.hikariMaximumPoolSize; } + /** Returns the idle timeout for HikariCP. */ + public static String getHibernateHikariIdleTimeout() { + return CONFIG_SETTINGS.get().hibernate.hikariIdleTimeout; + } + /** Returns the roid suffix to be used for the roids of all contacts and hosts. */ public static String getContactAndHostRoidSuffix() { return CONFIG_SETTINGS.get().registryPolicy.contactAndHostRoidSuffix; diff --git a/core/src/main/java/google/registry/config/RegistryConfigSettings.java b/core/src/main/java/google/registry/config/RegistryConfigSettings.java index 1471b6227..4c4405e12 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -112,7 +112,9 @@ public class RegistryConfigSettings { public String connectionIsolation; public String logSqlQueries; public String hikariConnectionTimeout; + public String hikariMinimumIdle; public String hikariMaximumPoolSize; + public String hikariIdleTimeout; } /** Configuration for Cloud SQL. */ diff --git a/core/src/main/java/google/registry/config/files/default-config.yaml b/core/src/main/java/google/registry/config/files/default-config.yaml index 3b3d1d58f..2485ed28a 100644 --- a/core/src/main/java/google/registry/config/files/default-config.yaml +++ b/core/src/main/java/google/registry/config/files/default-config.yaml @@ -207,17 +207,21 @@ hibernate: # Connection pool configurations. hikariConnectionTimeout: 20000 - # We occasionally received "Connection is not available, request timed out" # exception when setting minimumIdle to 0 and it turned out it is a bug (See - # https://github.com/brettwooldridge/HikariCP/issues/1212) in HikariCP and - # the workaround is to use a fixed size connection pool. + # https://github.com/brettwooldridge/HikariCP/issues/1212) in HikariCP. + # + # We tried to use a fixed size pool but ran into an issue(See b/155383029), + # so we need further investigation to figure out the proper size of the pool. # # HikariCP also recommends not setting minimumIdle for maximum performance # and responsiveness to spike demands (See # https://github.com/brettwooldridge/HikariCP). - # TODO(b/154720215): Experiment with a smaller pool size. - hikariMaximumPoolSize: 20 + # + # TODO(b/154720215): Investigate the long term fix. + hikariMinimumIdle: 1 + hikariMaximumPoolSize: 10 + hikariIdleTimeout: 300000 cloudSql: # jdbc url for the Cloud SQL database. diff --git a/core/src/main/java/google/registry/persistence/PersistenceModule.java b/core/src/main/java/google/registry/persistence/PersistenceModule.java index 294700137..511462cc3 100644 --- a/core/src/main/java/google/registry/persistence/PersistenceModule.java +++ b/core/src/main/java/google/registry/persistence/PersistenceModule.java @@ -17,7 +17,9 @@ package google.registry.persistence; import static com.google.common.base.Preconditions.checkState; import static google.registry.config.RegistryConfig.getHibernateConnectionIsolation; import static google.registry.config.RegistryConfig.getHibernateHikariConnectionTimeout; +import static google.registry.config.RegistryConfig.getHibernateHikariIdleTimeout; import static google.registry.config.RegistryConfig.getHibernateHikariMaximumPoolSize; +import static google.registry.config.RegistryConfig.getHibernateHikariMinimumIdle; import static google.registry.config.RegistryConfig.getHibernateLogSqlQueries; import com.google.api.client.auth.oauth2.Credential; @@ -48,7 +50,10 @@ public class PersistenceModule { // This name must be the same as the one defined in persistence.xml. public static final String PERSISTENCE_UNIT_NAME = "nomulus"; public static final String HIKARI_CONNECTION_TIMEOUT = "hibernate.hikari.connectionTimeout"; + public static final String HIKARI_MINIMUM_IDLE = "hibernate.hikari.minimumIdle"; public static final String HIKARI_MAXIMUM_POOL_SIZE = "hibernate.hikari.maximumPoolSize"; + public static final String HIKARI_IDLE_TIMEOUT = "hibernate.hikari.idleTimeout"; + public static final String HIKARI_DS_SOCKET_FACTORY = "hibernate.hikari.dataSource.socketFactory"; public static final String HIKARI_DS_CLOUD_SQL_INSTANCE = "hibernate.hikari.dataSource.cloudSqlInstance"; @@ -74,7 +79,9 @@ public class PersistenceModule { properties.put(Environment.ISOLATION, getHibernateConnectionIsolation()); properties.put(Environment.SHOW_SQL, getHibernateLogSqlQueries()); properties.put(HIKARI_CONNECTION_TIMEOUT, getHibernateHikariConnectionTimeout()); + properties.put(HIKARI_MINIMUM_IDLE, getHibernateHikariMinimumIdle()); properties.put(HIKARI_MAXIMUM_POOL_SIZE, getHibernateHikariMaximumPoolSize()); + properties.put(HIKARI_IDLE_TIMEOUT, getHibernateHikariIdleTimeout()); properties.put(Environment.DIALECT, NomulusPostgreSQLDialect.class.getName()); return properties.build(); } diff --git a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRule.java b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRule.java index 69b20e987..8b575e124 100644 --- a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRule.java +++ b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRule.java @@ -41,7 +41,6 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.HashMap; -import java.util.Map; import java.util.Optional; import java.util.Properties; import java.util.stream.Collectors; @@ -77,7 +76,7 @@ abstract class JpaTransactionManagerRule extends ExternalResource { private final Clock clock; private final Optional initScriptPath; private final ImmutableList extraEntityClasses; - private final ImmutableMap userProperties; + private final ImmutableMap userProperties; private static final JdbcDatabaseContainer database = create(); private static final HibernateSchemaExporter exporter = @@ -144,18 +143,15 @@ abstract class JpaTransactionManagerRule extends ExternalResource { new String(Files.readAllBytes(tempSqlFile.toPath()), StandardCharsets.UTF_8)); } - Map properties = - Maps.newHashMap(PersistenceModule.providesDefaultDatabaseConfigs()); - // Set the minimumIdle to 0 so assertReasonableNumDbConnections() can check if there is - // connection leak after each test. - properties.put("hibernate.hikari.minimumIdle", "0"); - properties.put("hibernate.hikari.idleTimeout", "300000"); + ImmutableMap properties = PersistenceModule.providesDefaultDatabaseConfigs(); if (!userProperties.isEmpty()) { // If there are user properties, create a new properties object with these added. - properties.putAll(userProperties); + ImmutableMap.Builder builder = properties.builder(); + builder.putAll(userProperties); // Forbid Hibernate push to stay consistent with flyway-based schema management. - properties.put(Environment.HBM2DDL_AUTO, "none"); - properties.put(Environment.SHOW_SQL, "true"); + builder.put(Environment.HBM2DDL_AUTO, "none"); + builder.put(Environment.SHOW_SQL, "true"); + properties = builder.build(); } assertReasonableNumDbConnections(); emf = @@ -163,7 +159,7 @@ abstract class JpaTransactionManagerRule extends ExternalResource { getJdbcUrl(), database.getUsername(), database.getPassword(), - ImmutableMap.copyOf(properties), + properties, extraEntityClasses); emfEntityHash = entityHash; }