Merge "Merge branch 'stable-2.14'"

Documentation/config-gerrit.txt

@@ -42,6 +42,53 @@ otherwise gerrit's init will remove the table.
   url = jdbc:postgresql://<host>:<port>/<db_name>?user=<user>&password=<password>
 ----
 
+[[accountPatchReviewDb.poolLimit]]accountPatchReviewDb.poolLimit::
++
+Maximum number of open database connections.  If the server needs
+more than this number, request processing threads will wait up
+to <<accountPatchReviewDb.poolMaxWait, poolMaxWait>> seconds for a
+connection to be released before they abort with an exception.
+This limit must be several units higher than the total number of
+httpd and sshd threads as some request processing code paths may
+need multiple connections.
++
+Default is <<sshd.threads, sshd.threads>>
+ + <<httpd.maxThreads, httpd.maxThreads>> + 2.
++
+
+[[accountPatchReviewDb.poolMinIdle]]database.poolMinIdle::
++
+Minimum number of connections to keep idle in the pool.
+Default is 4.
++
+
+[[accountPatchReviewDb.poolMaxIdle]]accountPatchReviewDb.poolMaxIdle::
++
+Maximum number of connections to keep idle in the pool.  If there
+are more idle connections, connections will be closed instead of
+being returned back to the pool.
+Default is min(<<accountPatchReviewDb.poolLimit, accountPatchReviewDb.poolLimit>>, 16).
++
+
+[[accountPatchReviewDb.poolMaxWait]]accountPatchReviewDb.poolMaxWait::
++
+Maximum amount of time a request processing thread will wait to
+acquire a database connection from the pool.  If no connection is
+released within this time period, the processing thread will abort
+its current operations and return an error to the client.
+Values should use common unit suffixes to express their setting:
++
+* ms, milliseconds
+* s, sec, second, seconds
+* m, min, minute, minutes
+* h, hr, hour, hours
+
++
+--
+If a unit suffix is not specified, `milliseconds` is assumed.
+
+Default is `30 seconds`.
+
 [[accounts]]
 === Section accounts
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateAccountPatchReviewDb.java

@@ -19,6 +19,7 @@ import com.google.common.base.Strings;
 import com.google.gerrit.pgm.util.SiteProgram;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gerrit.server.schema.DataSourceProvider;
 import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore;
 import com.google.inject.Injector;
@@ -47,15 +48,17 @@ public class MigrateAccountPatchReviewDb extends SiteProgram {
 
   @Override
   public int run() throws Exception {
+    Injector dbInjector = createDbInjector(DataSourceProvider.Context.SINGLE_USER);
     SitePaths sitePaths = new SitePaths(getSitePath());
+    ThreadSettingsConfig threadSettingsConfig = dbInjector.getInstance(ThreadSettingsConfig.class);
     Config fakeCfg = new Config();
     if (!Strings.isNullOrEmpty(sourceUrl)) {
       fakeCfg.setString("accountPatchReviewDb", null, "url", sourceUrl);
     }
     JdbcAccountPatchReviewStore sourceJdbcAccountPatchReviewStore =
-        JdbcAccountPatchReviewStore.createAccountPatchReviewStore(fakeCfg, sitePaths);
+        JdbcAccountPatchReviewStore.createAccountPatchReviewStore(
+            fakeCfg, sitePaths, threadSettingsConfig);
 
-    Injector dbInjector = createDbInjector(DataSourceProvider.Context.SINGLE_USER);
     Config cfg = dbInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
     String targetUrl = cfg.getString("accountPatchReviewDb", null, "url");
     if (targetUrl == null) {
@@ -64,7 +67,8 @@ public class MigrateAccountPatchReviewDb extends SiteProgram {
     }
     System.out.println("target Url: " + targetUrl);
     JdbcAccountPatchReviewStore targetJdbcAccountPatchReviewStore =
-        JdbcAccountPatchReviewStore.createAccountPatchReviewStore(cfg, sitePaths);
+        JdbcAccountPatchReviewStore.createAccountPatchReviewStore(
+            cfg, sitePaths, threadSettingsConfig);
     targetJdbcAccountPatchReviewStore.createTableIfNotExists();
 
     if (!isTargetTableEmpty(targetJdbcAccountPatchReviewStore)) {

gerrit-server/src/main/java/com/google/gerrit/server/schema/H2AccountPatchReviewStore.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.schema;
 
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -27,8 +28,11 @@ import org.eclipse.jgit.lib.Config;
 public class H2AccountPatchReviewStore extends JdbcAccountPatchReviewStore {
 
   @Inject
-  H2AccountPatchReviewStore(@GerritServerConfig Config cfg, SitePaths sitePaths) {
+  H2AccountPatchReviewStore(
-    super(cfg, sitePaths);
+      @GerritServerConfig Config cfg,
+      SitePaths sitePaths,
+      ThreadSettingsConfig threadSettingsConfig) {
+    super(cfg, sitePaths, threadSettingsConfig);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/schema/JdbcAccountPatchReviewStore.java

@@ -14,6 +14,9 @@
 
 package com.google.gerrit.server.schema;
 
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+
 import com.google.common.collect.ImmutableSet;
 import com.google.common.primitives.Ints;
 import com.google.gerrit.extensions.events.LifecycleListener;
@@ -22,8 +25,10 @@ import com.google.gerrit.lifecycle.LifecycleModule;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.change.AccountPatchReviewStore;
+import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import java.sql.Connection;
@@ -77,29 +82,30 @@ public abstract class JdbcAccountPatchReviewStore
     }
   }
 
-  private final DataSource ds;
+  private DataSource ds;
 
   public static JdbcAccountPatchReviewStore createAccountPatchReviewStore(
-      Config cfg, SitePaths sitePaths) {
+      Config cfg, SitePaths sitePaths, ThreadSettingsConfig threadSettingsConfig) {
     String url = cfg.getString(ACCOUNT_PATCH_REVIEW_DB, null, URL);
     if (url == null || url.contains(H2_DB)) {
-      return new H2AccountPatchReviewStore(cfg, sitePaths);
+      return new H2AccountPatchReviewStore(cfg, sitePaths, threadSettingsConfig);
     }
     if (url.contains(POSTGRESQL)) {
-      return new PostgresqlAccountPatchReviewStore(cfg, sitePaths);
+      return new PostgresqlAccountPatchReviewStore(cfg, sitePaths, threadSettingsConfig);
     }
     if (url.contains(MYSQL)) {
-      return new MysqlAccountPatchReviewStore(cfg, sitePaths);
+      return new MysqlAccountPatchReviewStore(cfg, sitePaths, threadSettingsConfig);
     }
     if (url.contains(MARIADB)) {
-      return new MariaDBAccountPatchReviewStore(cfg, sitePaths);
+      return new MariaDBAccountPatchReviewStore(cfg, sitePaths, threadSettingsConfig);
     }
     throw new IllegalArgumentException(
         "unsupported driver type for account patch reviews db: " + url);
   }
 
-  protected JdbcAccountPatchReviewStore(Config cfg, SitePaths sitePaths) {
+  protected JdbcAccountPatchReviewStore(
-    this.ds = createDataSource(getUrl(cfg, sitePaths));
+      Config cfg, SitePaths sitePaths, ThreadSettingsConfig threadSettingsConfig) {
+    this.ds = createDataSource(cfg, sitePaths, threadSettingsConfig);
   }
 
   protected JdbcAccountPatchReviewStore(DataSource ds) {
@@ -114,27 +120,45 @@ public abstract class JdbcAccountPatchReviewStore
     return url;
   }
 
-  protected static DataSource createDataSource(String url) {
+  private static DataSource createDataSource(
+      Config cfg, SitePaths sitePaths, ThreadSettingsConfig threadSettingsConfig) {
     BasicDataSource datasource = new BasicDataSource();
-    if (url.contains(POSTGRESQL)) {
+    String url = getUrl(cfg, sitePaths);
-      datasource.setDriverClassName("org.postgresql.Driver");
+    int poolLimit = threadSettingsConfig.getDatabasePoolLimit();
-    } else if (url.contains(H2_DB)) {
-      datasource.setDriverClassName("org.h2.Driver");
-    } else if (url.contains(MYSQL)) {
-      datasource.setDriverClassName("com.mysql.jdbc.Driver");
-    } else if (url.contains(MARIADB)) {
-      datasource.setDriverClassName("org.mariadb.jdbc.Driver");
-    }
     datasource.setUrl(url);
-    datasource.setMaxActive(50);
+    datasource.setDriverClassName(getDriverFromUrl(url));
-    datasource.setMinIdle(4);
+    datasource.setMaxActive(cfg.getInt(ACCOUNT_PATCH_REVIEW_DB, "poolLimit", poolLimit));
-    datasource.setMaxIdle(16);
+    datasource.setMinIdle(cfg.getInt(ACCOUNT_PATCH_REVIEW_DB, "poolminidle", 4));
-    long evictIdleTimeMs = 1000 * 60;
+    datasource.setMaxIdle(
+        cfg.getInt(ACCOUNT_PATCH_REVIEW_DB, "poolmaxidle", Math.min(poolLimit, 16)));
+    datasource.setInitialSize(datasource.getMinIdle());
+    datasource.setMaxWait(
+        ConfigUtil.getTimeUnit(
+            cfg,
+            ACCOUNT_PATCH_REVIEW_DB,
+            null,
+            "poolmaxwait",
+            MILLISECONDS.convert(30, SECONDS),
+            MILLISECONDS));
+    long evictIdleTimeMs = 1000L * 60;
     datasource.setMinEvictableIdleTimeMillis(evictIdleTimeMs);
     datasource.setTimeBetweenEvictionRunsMillis(evictIdleTimeMs / 2);
     return datasource;
   }
 
+  private static String getDriverFromUrl(String url) {
+    if (url.contains(POSTGRESQL)) {
+      return "org.postgresql.Driver";
+    }
+    if (url.contains(MYSQL)) {
+      return "com.mysql.jdbc.Driver";
+    }
+    if (url.contains(MARIADB)) {
+      return "org.mariadb.jdbc.Driver";
+    }
+    return "org.h2.Driver";
+  }
+
   @Override
   public void start() {
     try {

gerrit-server/src/main/java/com/google/gerrit/server/schema/MariaDBAccountPatchReviewStore.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.schema;
 
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -27,8 +28,11 @@ import org.eclipse.jgit.lib.Config;
 public class MariaDBAccountPatchReviewStore extends JdbcAccountPatchReviewStore {
 
   @Inject
-  MariaDBAccountPatchReviewStore(@GerritServerConfig Config cfg, SitePaths sitePaths) {
+  MariaDBAccountPatchReviewStore(
-    super(cfg, sitePaths);
+      @GerritServerConfig Config cfg,
+      SitePaths sitePaths,
+      ThreadSettingsConfig threadSettingsConfig) {
+    super(cfg, sitePaths, threadSettingsConfig);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/schema/MysqlAccountPatchReviewStore.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.schema;
 
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -27,8 +28,11 @@ import org.eclipse.jgit.lib.Config;
 public class MysqlAccountPatchReviewStore extends JdbcAccountPatchReviewStore {
 
   @Inject
-  MysqlAccountPatchReviewStore(@GerritServerConfig Config cfg, SitePaths sitePaths) {
+  MysqlAccountPatchReviewStore(
-    super(cfg, sitePaths);
+      @GerritServerConfig Config cfg,
+      SitePaths sitePaths,
+      ThreadSettingsConfig threadSettingsConfig) {
+    super(cfg, sitePaths, threadSettingsConfig);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/schema/PostgresqlAccountPatchReviewStore.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.schema;
 
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -27,8 +28,11 @@ import org.eclipse.jgit.lib.Config;
 public class PostgresqlAccountPatchReviewStore extends JdbcAccountPatchReviewStore {
 
   @Inject
-  PostgresqlAccountPatchReviewStore(@GerritServerConfig Config cfg, SitePaths sitePaths) {
+  PostgresqlAccountPatchReviewStore(
-    super(cfg, sitePaths);
+      @GerritServerConfig Config cfg,
+      SitePaths sitePaths,
+      ThreadSettingsConfig threadSettingsConfig) {
+    super(cfg, sitePaths, threadSettingsConfig);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_127.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.schema;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.config.ThreadSettingsConfig;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -32,18 +33,25 @@ public class Schema_127 extends SchemaVersion {
 
   private final SitePaths sitePaths;
   private final Config cfg;
+  private final ThreadSettingsConfig threadSettingsConfig;
 
   @Inject
-  Schema_127(Provider<Schema_126> prior, SitePaths sitePaths, @GerritServerConfig Config cfg) {
+  Schema_127(
+      Provider<Schema_126> prior,
+      SitePaths sitePaths,
+      @GerritServerConfig Config cfg,
+      ThreadSettingsConfig threadSettingsConfig) {
     super(prior);
     this.sitePaths = sitePaths;
     this.cfg = cfg;
+    this.threadSettingsConfig = threadSettingsConfig;
   }
 
   @Override
   protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
     JdbcAccountPatchReviewStore jdbcAccountPatchReviewStore =
-        JdbcAccountPatchReviewStore.createAccountPatchReviewStore(cfg, sitePaths);
+        JdbcAccountPatchReviewStore.createAccountPatchReviewStore(
+            cfg, sitePaths, threadSettingsConfig);
     jdbcAccountPatchReviewStore.dropTableIfExists();
     jdbcAccountPatchReviewStore.createTableIfNotExists();
     try (Connection con = jdbcAccountPatchReviewStore.getConnection();