From 8960a9edce35b4dfcae59b74271ccf1775b7b081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Fri, 17 Apr 2015 11:25:55 -0400 Subject: [PATCH 1/2] Fix gc_log when running in a web container All logs supposed to be in gc_log file were ending up in main log instead when deploying Gerrit in a web container. gc_log was setup only when running Gerrit as a daemon. Rework GarbageCollectionFile class to be able to get sitePaths as a dependency and create a module to load it in WebAppInitializer. Change-Id: Iee58a1ca36d62060d8f844c00254012db16705e2 --- .../java/com/google/gerrit/pgm/Daemon.java | 2 +- .../pgm/util/GarbageCollectionLogFile.java | 35 +++++++++++-------- .../gerrit/httpd/WebAppInitializer.java | 2 ++ 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index f5133483a6..89fa86bebb 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -195,7 +195,6 @@ public class Daemon extends SiteProgram { throw die("No services enabled, nothing to do"); } - manager.add(GarbageCollectionLogFile.start(getSitePath())); if (consoleLog) { } else { manager.add(ErrorLogFile.start(getSitePath())); @@ -364,6 +363,7 @@ public class Daemon extends SiteProgram { } } }); + modules.add(new GarbageCollectionLogFile.Module()); modules.add(GarbageCollectionRunner.module()); return cfgInjector.createChildInjector(modules); } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java index 7d33a36cc2..9274e78240 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java @@ -16,39 +16,46 @@ package com.google.gerrit.pgm.util; import com.google.gerrit.common.Die; import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.GarbageCollection; import com.google.gerrit.server.util.SystemLog; +import com.google.inject.Inject; import org.apache.log4j.LogManager; import org.apache.log4j.Logger; import org.apache.log4j.PatternLayout; import java.io.File; -import java.io.FileNotFoundException; -public class GarbageCollectionLogFile { +public class GarbageCollectionLogFile implements LifecycleListener { - public static LifecycleListener start(File sitePath) - throws FileNotFoundException { - File logdir = new SitePaths(sitePath).logs_dir; + public static class Module extends LifecycleModule { + @Override + protected void configure() { + bind(GarbageCollectionLogFile.class).asEagerSingleton(); + listener().to(GarbageCollectionLogFile.class); + } + } + + @Inject + public GarbageCollectionLogFile(SitePaths sitePaths) { + File logdir = sitePaths.logs_dir; if (!logdir.exists() && !logdir.mkdirs()) { throw new Die("Cannot create log directory: " + logdir); } if (SystemLog.shouldConfigure()) { initLogSystem(logdir); } + } - return new LifecycleListener() { - @Override - public void start() { - } + @Override + public void start() { + } - @Override - public void stop() { - LogManager.getLogger(GarbageCollection.LOG_NAME).removeAllAppenders(); - } - }; + @Override + public void stop() { + LogManager.getLogger(GarbageCollection.LOG_NAME).removeAllAppenders(); } private static void initLogSystem(File logdir) { diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index c579f45728..9c59be0dcf 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -25,6 +25,7 @@ import com.google.gerrit.httpd.plugins.HttpPluginModule; import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.lucene.LuceneIndexModule; +import com.google.gerrit.pgm.util.GarbageCollectionLogFile; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; @@ -319,6 +320,7 @@ public class WebAppInitializer extends GuiceServletContextListener bind(GerritOptions.class).toInstance(new GerritOptions(false, false)); } }); + modules.add(new GarbageCollectionLogFile.Module()); modules.add(GarbageCollectionRunner.module()); return cfgInjector.createChildInjector(modules); } From 146afb82f0a26554b7e5fba3a41438486115d850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Fri, 17 Apr 2015 12:25:52 -0400 Subject: [PATCH 2/2] Move all GarbageCollection bindings into the same module Also move the GarbageCollectionLogFile class in the same package as the other classes binded in that module. Change-Id: I2ca9e097b51181509d9773d9fbf299ed26dc4330 --- .../java/com/google/gerrit/pgm/Daemon.java | 6 ++-- .../server/config/GerritGlobalModule.java | 2 -- .../server/git}/GarbageCollectionLogFile.java | 12 +------- .../server/git/GarbageCollectionModule.java | 30 +++++++++++++++++++ .../server/git/GarbageCollectionRunner.java | 12 -------- .../gerrit/testutil/InMemoryModule.java | 2 ++ .../gerrit/httpd/WebAppInitializer.java | 6 ++-- 7 files changed, 37 insertions(+), 33 deletions(-) rename {gerrit-pgm/src/main/java/com/google/gerrit/pgm/util => gerrit-server/src/main/java/com/google/gerrit/server/git}/GarbageCollectionLogFile.java (83%) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionModule.java diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index 89fa86bebb..ae29ff76f8 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -37,7 +37,6 @@ import com.google.gerrit.pgm.http.jetty.JettyEnv; import com.google.gerrit.pgm.http.jetty.JettyModule; import com.google.gerrit.pgm.http.jetty.ProjectQoSFilter; import com.google.gerrit.pgm.util.ErrorLogFile; -import com.google.gerrit.pgm.util.GarbageCollectionLogFile; import com.google.gerrit.pgm.util.LogFileCompressor; import com.google.gerrit.pgm.util.RuntimeShutdown; import com.google.gerrit.pgm.util.SiteProgram; @@ -54,7 +53,7 @@ import com.google.gerrit.server.config.MasterNodeStartup; import com.google.gerrit.server.config.RestCacheAdminModule; import com.google.gerrit.server.contact.ContactStoreModule; import com.google.gerrit.server.contact.HttpContactStoreConnection; -import com.google.gerrit.server.git.GarbageCollectionRunner; +import com.google.gerrit.server.git.GarbageCollectionModule; import com.google.gerrit.server.git.ReceiveCommitsExecutorModule; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.index.DummyIndexModule; @@ -363,8 +362,7 @@ public class Daemon extends SiteProgram { } } }); - modules.add(new GarbageCollectionLogFile.Module()); - modules.add(GarbageCollectionRunner.module()); + modules.add(new GarbageCollectionModule()); return cfgInjector.createChildInjector(modules); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 1dff046890..91312b3c11 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -74,7 +74,6 @@ import com.google.gerrit.server.change.MergeabilityCacheImpl; import com.google.gerrit.server.events.EventFactory; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.ChangeMergeQueue; -import com.google.gerrit.server.git.GarbageCollection; import com.google.gerrit.server.git.GitModule; import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.git.MergeUtil; @@ -206,7 +205,6 @@ public class GerritGlobalModule extends FactoryModule { factory(RegisterNewEmailSender.Factory.class); factory(ReplacePatchSetSender.Factory.class); factory(PerformCreateProject.Factory.class); - factory(GarbageCollection.Factory.class); bind(PermissionCollection.Factory.class); bind(AccountVisibility.class) .toProvider(AccountVisibilityProvider.class) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java similarity index 83% rename from gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java rename to gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java index 9274e78240..765914603c 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/GarbageCollectionLogFile.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionLogFile.java @@ -12,13 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.gerrit.pgm.util; +package com.google.gerrit.server.git; import com.google.gerrit.common.Die; import com.google.gerrit.extensions.events.LifecycleListener; -import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.server.config.SitePaths; -import com.google.gerrit.server.git.GarbageCollection; import com.google.gerrit.server.util.SystemLog; import com.google.inject.Inject; @@ -30,14 +28,6 @@ import java.io.File; public class GarbageCollectionLogFile implements LifecycleListener { - public static class Module extends LifecycleModule { - @Override - protected void configure() { - bind(GarbageCollectionLogFile.class).asEagerSingleton(); - listener().to(GarbageCollectionLogFile.class); - } - } - @Inject public GarbageCollectionLogFile(SitePaths sitePaths) { File logdir = sitePaths.logs_dir; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionModule.java new file mode 100644 index 0000000000..aacc738211 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionModule.java @@ -0,0 +1,30 @@ +// Copyright (C) 2015 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 com.google.gerrit.lifecycle.LifecycleModule; + +public class GarbageCollectionModule extends LifecycleModule { + + @Override + protected void configure() { + bind(GarbageCollectionLogFile.class).asEagerSingleton(); + listener().to(GarbageCollectionLogFile.class); + + bind(GarbageCollectionQueue.class); + factory(GarbageCollection.Factory.class); + listener().to(GarbageCollectionRunner.Lifecycle.class); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionRunner.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionRunner.java index 5e3ca317d8..68d25d9a19 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionRunner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollectionRunner.java @@ -18,12 +18,10 @@ import static com.google.gerrit.server.config.ScheduleConfig.MISSING_CONFIG; import com.google.common.collect.Lists; import com.google.gerrit.extensions.events.LifecycleListener; -import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.server.config.GcConfig; import com.google.gerrit.server.config.ScheduleConfig; import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; -import com.google.inject.Module; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,16 +33,6 @@ public class GarbageCollectionRunner implements Runnable { private static final Logger gcLog = LoggerFactory .getLogger(GarbageCollection.LOG_NAME); - public static Module module() { - return new LifecycleModule() { - - @Override - protected void configure() { - listener().to(Lifecycle.class); - } - }; - } - static class Lifecycle implements LifecycleListener { private final WorkQueue queue; private final GarbageCollectionRunner gcRunner; diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java index fc74c207db..dcfadd45ba 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java @@ -42,6 +42,7 @@ import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFootersProvider; import com.google.gerrit.server.git.EmailReviewCommentsExecutor; +import com.google.gerrit.server.git.GarbageCollection; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.PerThreadRequestScope; import com.google.gerrit.server.git.WorkQueue; @@ -125,6 +126,7 @@ public class InMemoryModule extends FactoryModule { } }); install(cfgInjector.getInstance(GerritGlobalModule.class)); + factory(GarbageCollection.Factory.class); bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST); diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 9c59be0dcf..99e545049d 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -25,7 +25,6 @@ import com.google.gerrit.httpd.plugins.HttpPluginModule; import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.lucene.LuceneIndexModule; -import com.google.gerrit.pgm.util.GarbageCollectionLogFile; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; @@ -40,7 +39,7 @@ import com.google.gerrit.server.config.RestCacheAdminModule; import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.contact.ContactStoreModule; import com.google.gerrit.server.contact.HttpContactStoreConnection; -import com.google.gerrit.server.git.GarbageCollectionRunner; +import com.google.gerrit.server.git.GarbageCollectionModule; import com.google.gerrit.server.git.LocalDiskRepositoryManager; import com.google.gerrit.server.git.ReceiveCommitsExecutorModule; import com.google.gerrit.server.git.WorkQueue; @@ -320,8 +319,7 @@ public class WebAppInitializer extends GuiceServletContextListener bind(GerritOptions.class).toInstance(new GerritOptions(false, false)); } }); - modules.add(new GarbageCollectionLogFile.Module()); - modules.add(GarbageCollectionRunner.module()); + modules.add(new GarbageCollectionModule()); return cfgInjector.createChildInjector(modules); }