Fix: The email notification of review comments gets stuck.

Sometimes it is found that one thread goes stuck when waiting
for an answer from the SMTP server.

Fixed. Enable user to

 -config the timeout value of opening a socket connected to a remote
  SMTP server.

 -config the worker-thread pool size of executor used for sending out
  review comments notification when it is not enough to dedicate only
  one thread.

 -config the default size of the background execution thread pool when
  one thread is not enough to handle miscellaneous tasks including
  sending out every kind email notification.

Change-Id: Id8177b374f7049cfac617c50e66b2c83ae71641b

Documentation/config-gerrit.txt

@@ -2412,6 +2412,16 @@ only the default internal rules will be used.
 +
 Default is true, to execute project specific rules.
 
+[[execution]]Section execution
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+[[execution.defaultThreadPoolSize]]execution.defaultThreadPoolSize::
++
+The default size of the background execution thread pool in
+which miscellaneous tasks are handled.
++
+Default is 1.
+
 [[sendemail]]Section sendemail
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -2422,6 +2432,26 @@ and all other properties of section sendemail are ignored.
 +
 By default, true, allowing notifications to be sent.
 
+[[sendemail.connectTimeout]]sendemail.connectTimeout::
++
+The connection timeout of opening a socket connected to a
+remote SMTP server.
++
+Values can be specified using standard time unit abbreviations
+('ms', 'sec', 'min', etc.).
+If no unit is specified, milliseconds is assumed.
++
+Default is 0. A timeout of zero is interpreted as an infinite
+timeout. The connection will then block until established or
+an error occurs.
+
+[[sendemail.threadPoolSize]]sendemail.threadPoolSize::
++
+Maximum size of thread pool in which the review comments
+notifications are sent out asynchronously.
++
+By default, 1.
+
 [[sendemail.from]]sendemail.from::
 +
 Designates what name and address Gerrit will place in the From

gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java

@@ -23,7 +23,8 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.change.PostReview.NotifyHandling;
-import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.git.EmailReviewCommentsExecutor;
+import com.google.gerrit.server.git.WorkQueue.Executor;
 import com.google.gerrit.server.mail.CommentSender;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.util.RequestContext;
@@ -55,7 +56,7 @@ class EmailReviewComments implements Runnable, RequestContext {
         List<PatchLineComment> comments);
   }
 
-  private final WorkQueue workQueue;
+  private final Executor sendEmailsExecutor;
   private final PatchSetInfoFactory patchSetInfoFactory;
   private final CommentSender.Factory commentSenderFactory;
   private final SchemaFactory<ReviewDb> schemaFactory;
@@ -71,7 +72,7 @@ class EmailReviewComments implements Runnable, RequestContext {
 
   @Inject
   EmailReviewComments (
-      WorkQueue workQueue,
+      @EmailReviewCommentsExecutor final Executor executor,
       PatchSetInfoFactory patchSetInfoFactory,
       CommentSender.Factory commentSenderFactory,
       SchemaFactory<ReviewDb> schemaFactory,
@@ -82,7 +83,7 @@ class EmailReviewComments implements Runnable, RequestContext {
       @Assisted Account.Id authorId,
       @Assisted ChangeMessage message,
       @Assisted List<PatchLineComment> comments) {
-    this.workQueue = workQueue;
+    this.sendEmailsExecutor = executor;
     this.patchSetInfoFactory = patchSetInfoFactory;
     this.commentSenderFactory = commentSenderFactory;
     this.schemaFactory = schemaFactory;
@@ -96,7 +97,7 @@ class EmailReviewComments implements Runnable, RequestContext {
   }
 
   void sendAsync() {
-    workQueue.getDefaultQueue().submit(this);
+    sendEmailsExecutor.submit(this);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java

@@ -0,0 +1,30 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+
+import java.lang.annotation.Retention;
+
+/**
+ * Marker on the global {@link WorkQueue.Executor} used by
+ * {@link EmailReviewComments}.
+ */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface EmailReviewCommentsExecutor {
+}

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java

@@ -45,6 +45,15 @@ public class ReceiveCommitsExecutorModule extends AbstractModule {
     return queues.createQueue(poolSize, "ReceiveCommits");
   }
 
+  @Provides
+  @Singleton
+  @EmailReviewCommentsExecutor
+  public WorkQueue.Executor createEmailReviewCommentsExecutor(
+      @GerritServerConfig Config config, WorkQueue queues) {
+    int poolSize = config.getInt("sendemail", null, "threadPoolSize", 1);
+    return queues.createQueue(poolSize, "EmailReviewComments");
+  }
+
   @Provides
   @Singleton
   @ChangeUpdateExecutor

gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java

@@ -18,10 +18,12 @@ import com.google.common.collect.Lists;
 import com.google.gerrit.extensions.events.LifecycleListener;
 import com.google.gerrit.lifecycle.LifecycleModule;
 import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.util.IdGenerator;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
+import org.eclipse.jgit.lib.Config;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -83,19 +85,21 @@ public class WorkQueue {
       };
 
   private Executor defaultQueue;
+  private int defaultQueueSize;
   private final IdGenerator idGenerator;
   private final CopyOnWriteArrayList<Executor> queues;
 
   @Inject
-  WorkQueue(final IdGenerator idGenerator) {
+  WorkQueue(final IdGenerator idGenerator, @GerritServerConfig final Config cfg) {
     this.idGenerator = idGenerator;
     this.queues = new CopyOnWriteArrayList<Executor>();
+    defaultQueueSize = cfg.getInt("execution", "defaultThreadPoolSize", 1);
   }
 
   /** Get the default work queue, for miscellaneous tasks. */
   public synchronized Executor getDefaultQueue() {
     if (defaultQueue == null) {
-      defaultQueue = createQueue(1, "WorkQueue");
+      defaultQueue = createQueue(defaultQueueSize, "WorkQueue");
     }
     return defaultQueue;
   }

gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.mail;
 
+import com.google.common.primitives.Ints;
 import com.google.gerrit.common.Version;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.server.config.ConfigUtil;
@@ -39,10 +40,14 @@ import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 /** Sends email via a nearby SMTP server. */
 @Singleton
 public class SmtpEmailSender implements EmailSender {
+  /** The socket's connect timeout (0 = infinite timeout) */
+  private static final int DEFAULT_CONNECT_TIMEOUT = 0;
+
   public static class Module extends AbstractModule {
     @Override
     protected void configure() {
@@ -55,6 +60,7 @@ public class SmtpEmailSender implements EmailSender {
   }
 
   private final boolean enabled;
+  private final int connectTimeout;
 
   private String smtpHost;
   private int smtpPort;
@@ -69,6 +75,10 @@ public class SmtpEmailSender implements EmailSender {
   @Inject
   SmtpEmailSender(@GerritServerConfig final Config cfg) {
     enabled = cfg.getBoolean("sendemail", null, "enable", true);
+    connectTimeout =
+        Ints.checkedCast(ConfigUtil.getTimeUnit(cfg, "sendemail", null,
+            "connectTimeout", DEFAULT_CONNECT_TIMEOUT, TimeUnit.MILLISECONDS));
+
 
     smtpHost = cfg.getString("sendemail", null, "smtpserver");
     if (smtpHost == null) {
@@ -239,6 +249,7 @@ public class SmtpEmailSender implements EmailSender {
     }
 
     try {
+      client.setConnectTimeout(connectTimeout);
       client.connect(smtpHost, smtpPort);
       if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
         throw new EmailException("SMTP server rejected connection");