opendev/gerrit
Code Issues Proposed changes

Make async logging buffer size configurable.

Browse Source 
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 <edwin.kempin@sap.com>
Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com>
This commit is contained in:
Sasa Zivkov
2012-06-11 14:05:14 +02:00
parent c347a0b534
commit f69aeb124d
4 changed files with 17 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
Documentation/config-gerrit.txt
View File

@@ -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
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

5
gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
View File

@@ -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();

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
View File

@@ -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;
}

6
gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java
View File

@@ -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<SshSession> session, final Provider<Context> 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();