From f69aeb124dc3acc64be1e164a606a1df1f6b1426 Mon Sep 17 00:00:00 2001 From: Sasa Zivkov Date: Mon, 11 Jun 2012 14:05:14 +0200 Subject: [PATCH] Make async logging buffer size configurable. Introduce the core.asyncLogginBufferSize parameter as size of the buffer to store logging events for asynchronous logging. Putting a larger value can protect threads from stalling when the AsyncAppender threads are not fast enough to consume the logging events from the buffer. It also protects from loosing log entries in this case. In addition, instead of expected thread stall when the logging buffer is full there is a bad side effect where an AsyncAppender-Dispatcher thread dies which then causes all NioProcessor threads to block on AsyncAppender.append method. Finally Gerrit stops responding on SSH requests. The reason for AsyncAppender-Dispatcher thread death is that it tries to discard a set of unprocessed logging events from the buffer but throws a NPE because of trying to compute hash value on the logger parameter of the LogginEvent which is set to null from Gerrit. We should probably also try to make our logging events discardable. This is not part of this change. Change-Id: I3fe97a7a0827293a385bf6ccb46ed2f4af53b674 Signed-off-by: Edwin Kempin Signed-off-by: Sasa Zivkov --- Documentation/config-gerrit.txt | 9 +++++++++ .../java/com/google/gerrit/pgm/http/jetty/HttpLog.java | 5 +++-- .../com/google/gerrit/pgm/http/jetty/JettyServer.java | 2 +- .../src/main/java/com/google/gerrit/sshd/SshLog.java | 6 ++++-- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 54f77525e2..535aaa8036 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -847,6 +847,15 @@ unused mapped spaces fast enough. Default on JGit is false. Although potentially slower, it yields much more predictable behavior. +[[core.asyncLoggingBufferSize]]core.asyncLoggingBufferSize:: ++ +Size of the buffer to store logging events for asynchronous logging. +Putting a larger value can protect threads from stalling when the +AsyncAppender threads are not fast enough to consume the logging events +from the buffer. It also protects from loosing log entries in this case. ++ +Default is 64 entries. + [[database]]Section database ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HttpLog.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HttpLog.java index 65e73bd739..999f16456d 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HttpLog.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HttpLog.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.RequestLog; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jgit.lib.Config; import java.io.File; import java.io.IOException; @@ -54,7 +55,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog { private final AsyncAppender async; - HttpLog(final SitePaths site) { + HttpLog(final SitePaths site, final Config config) { final DailyRollingFileAppender dst = new DailyRollingFileAppender(); dst.setName(LOG_NAME); dst.setLayout(new MyLayout()); @@ -69,7 +70,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog { async = new AsyncAppender(); async.setBlocking(true); - async.setBufferSize(64); + async.setBufferSize(config.getInt("core", "asyncLoggingBufferSize", 64)); async.setLocationInfo(false); async.addAppender(dst); async.activateOptions(); diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java index fa5ef5918d..0a0a3cc6fd 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java @@ -116,7 +116,7 @@ public class JettyServer { Handler app = makeContext(env, cfg); if (cfg.getBoolean("httpd", "requestlog", !reverseProxy)) { RequestLogHandler handler = new RequestLogHandler(); - handler.setRequestLog(new HttpLog(site)); + handler.setRequestLog(new HttpLog(site, cfg)); handler.setHandler(app); app = handler; } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java index 32d5a076c6..40ddb85083 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java @@ -18,6 +18,7 @@ import com.google.gerrit.lifecycle.LifecycleListener; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PeerDaemonUser; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.util.IdGenerator; import com.google.gerrit.sshd.SshScope.Context; @@ -33,6 +34,7 @@ import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.apache.log4j.spi.ErrorHandler; import org.apache.log4j.spi.LoggingEvent; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.util.QuotedString; import java.io.File; @@ -59,7 +61,7 @@ class SshLog implements LifecycleListener { @Inject SshLog(final Provider session, final Provider context, - final SitePaths site) { + final SitePaths site, @GerritServerConfig Config config) { this.session = session; this.context = context; @@ -77,7 +79,7 @@ class SshLog implements LifecycleListener { async = new AsyncAppender(); async.setBlocking(true); - async.setBufferSize(64); + async.setBufferSize(config.getInt("core", "asyncLoggingBufferSize", 64)); async.setLocationInfo(false); async.addAppender(dst); async.activateOptions();