diff --git a/core/src/main/java/google/registry/config/RegistryConfig.java b/core/src/main/java/google/registry/config/RegistryConfig.java index a876daee5..16a403682 100644 --- a/core/src/main/java/google/registry/config/RegistryConfig.java +++ b/core/src/main/java/google/registry/config/RegistryConfig.java @@ -1512,21 +1512,11 @@ 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 4c4405e12..1471b6227 100644 --- a/core/src/main/java/google/registry/config/RegistryConfigSettings.java +++ b/core/src/main/java/google/registry/config/RegistryConfigSettings.java @@ -112,9 +112,7 @@ 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 4005fff5f..3b3d1d58f 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,9 +207,17 @@ hibernate: # Connection pool configurations. hikariConnectionTimeout: 20000 - hikariMinimumIdle: 0 + + # 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. + # + # 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 - 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 511462cc3..294700137 100644 --- a/core/src/main/java/google/registry/persistence/PersistenceModule.java +++ b/core/src/main/java/google/registry/persistence/PersistenceModule.java @@ -17,9 +17,7 @@ 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; @@ -50,10 +48,7 @@ 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"; @@ -79,9 +74,7 @@ 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 8b575e124..69b20e987 100644 --- a/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRule.java +++ b/core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerRule.java @@ -41,6 +41,7 @@ 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; @@ -76,7 +77,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 = @@ -143,15 +144,18 @@ abstract class JpaTransactionManagerRule extends ExternalResource { new String(Files.readAllBytes(tempSqlFile.toPath()), StandardCharsets.UTF_8)); } - ImmutableMap properties = PersistenceModule.providesDefaultDatabaseConfigs(); + 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"); if (!userProperties.isEmpty()) { // If there are user properties, create a new properties object with these added. - ImmutableMap.Builder builder = properties.builder(); - builder.putAll(userProperties); + properties.putAll(userProperties); // Forbid Hibernate push to stay consistent with flyway-based schema management. - builder.put(Environment.HBM2DDL_AUTO, "none"); - builder.put(Environment.SHOW_SQL, "true"); - properties = builder.build(); + properties.put(Environment.HBM2DDL_AUTO, "none"); + properties.put(Environment.SHOW_SQL, "true"); } assertReasonableNumDbConnections(); emf = @@ -159,7 +163,7 @@ abstract class JpaTransactionManagerRule extends ExternalResource { getJdbcUrl(), database.getUsername(), database.getPassword(), - properties, + ImmutableMap.copyOf(properties), extraEntityClasses); emfEntityHash = entityHash; }