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
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index c5ef758..038d946 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2412,6 +2412,16 @@
+
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 @@
+
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
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
index 11a9edd..eea2f48 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
@@ -23,7 +23,8 @@
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 @@
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 @@
@Inject
EmailReviewComments (
- WorkQueue workQueue,
+ @EmailReviewCommentsExecutor final Executor executor,
PatchSetInfoFactory patchSetInfoFactory,
CommentSender.Factory commentSenderFactory,
SchemaFactory<ReviewDb> schemaFactory,
@@ -82,7 +83,7 @@
@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 @@
}
void sendAsync() {
- workQueue.getDefaultQueue().submit(this);
+ sendEmailsExecutor.submit(this);
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java
new file mode 100644
index 0000000..1581ec48
--- /dev/null
+++ b/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 {
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java
index 1cbd227..81ce05d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java
@@ -47,6 +47,15 @@
@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
public ListeningExecutorService createChangeUpdateExecutor(@GerritServerConfig Config config) {
int poolSize = config.getInt("receive", null, "changeUpdateThreads", 1);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java
index 90c5335..d7a2cf4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java
@@ -18,10 +18,12 @@
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 @@
};
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;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
index 89fc6b9..48e2d03 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java
+++ b/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.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 @@
}
private final boolean enabled;
+ private final int connectTimeout;
private String smtpHost;
private int smtpPort;
@@ -69,6 +75,10 @@
@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 @@
}
try {
+ client.setConnectTimeout(connectTimeout);
client.connect(smtpHost, smtpPort);
if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
throw new EmailException("SMTP server rejected connection");
