diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java index e18d840d26..7018a3b7f5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java @@ -40,6 +40,8 @@ class CleanupHandle extends WeakReference { } if (!tmpFile.delete() && tmpFile.exists()) { PluginLoader.log.warn("Cannot delete " + tmpFile.getAbsolutePath()); + } else { + PluginLoader.log.info("Cleaned plugin " + tmpFile.getName()); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginCleanerTask.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginCleanerTask.java new file mode 100644 index 0000000000..7081d706d7 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginCleanerTask.java @@ -0,0 +1,100 @@ +// Copyright (C) 2012 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.plugins; + +import com.google.gerrit.server.git.WorkQueue; +import com.google.inject.Inject; +import com.google.inject.Singleton; + +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +@Singleton +class PluginCleanerTask implements Runnable { + private final WorkQueue workQueue; + private final PluginLoader loader; + private volatile int pending; + private Future self; + private int attempts; + private long start; + + @Inject + PluginCleanerTask(WorkQueue workQueue, PluginLoader loader) { + this.workQueue = workQueue; + this.loader = loader; + } + + @Override + public void run() { + try { + for (int t = 0; t < 2 * (attempts + 1); t++) { + System.gc(); + Thread.sleep(50); + } + } catch (InterruptedException e) { + } + + int left = loader.processPendingCleanups(); + synchronized (this) { + pending = left; + self = null; + + if (0 < left) { + long waiting = System.currentTimeMillis() - start; + PluginLoader.log.warn(String.format( + "%d plugins still waiting to be reclaimed after %d minutes", + pending, + TimeUnit.MILLISECONDS.toMinutes(waiting))); + attempts = Math.min(attempts + 1, 15); + ensureScheduled(); + } else { + attempts = 0; + } + } + } + + @Override + public String toString() { + int p = pending; + if (0 < p) { + return String.format("Plugin Cleaner (waiting for %d plugins)", p); + } + return "Plugin Cleaner"; + } + + synchronized void clean(int expect) { + if (self == null && pending == 0) { + start = System.currentTimeMillis(); + } + pending = expect; + ensureScheduled(); + } + + private void ensureScheduled() { + if (self == null && 0 < pending) { + if (attempts == 1) { + self = workQueue.getDefaultQueue().schedule( + this, + 30, + TimeUnit.SECONDS); + } else { + self = workQueue.getDefaultQueue().schedule( + this, + attempts + 1, + TimeUnit.MINUTES); + } + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java index 828a88d33c..324e3de433 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java @@ -26,6 +26,7 @@ import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; import com.google.inject.Module; +import com.google.inject.Provider; import com.google.inject.Singleton; import org.eclipse.jgit.lib.Config; @@ -68,12 +69,14 @@ public class PluginLoader implements LifecycleListener { private final Map broken; private final ReferenceQueue cleanupQueue; private final ConcurrentMap cleanupHandles; + private final Provider cleaner; private final PluginScannerThread scanner; @Inject public PluginLoader(SitePaths sitePaths, PluginGuiceEnvironment pe, ServerInformationImpl sii, + Provider pct, @GerritServerConfig Config cfg) { pluginsDir = sitePaths.plugins_dir; dataDir = sitePaths.data_dir; @@ -84,6 +87,7 @@ public class PluginLoader implements LifecycleListener { broken = Maps.newHashMap(); cleanupQueue = new ReferenceQueue(); cleanupHandles = Maps.newConcurrentMap(); + cleaner = pct; long checkFrequency = ConfigUtil.getTimeUnit(cfg, "plugins", null, "checkFrequency", @@ -110,7 +114,6 @@ public class PluginLoader implements LifecycleListener { File old = new File(pluginsDir, ".last_" + name + ".zip"); File tmp = asTemp(in, ".next_" + name, ".zip", pluginsDir); - boolean clean = false; synchronized (this) { Plugin active = running.get(name); if (active != null) { @@ -125,18 +128,13 @@ public class PluginLoader implements LifecycleListener { runPlugin(name, jar, active); if (active == null) { log.info(String.format("Installed plugin %s", name)); - } else { - clean = true; } } catch (PluginInstallException e) { jar.delete(); throw e; } - } - if (clean) { - System.gc(); - processPendingCleanups(); + cleanInBackground(); } } @@ -166,7 +164,6 @@ public class PluginLoader implements LifecycleListener { } public void disablePlugins(Set names) { - boolean clean = false; synchronized (this) { for (String name : names) { Plugin active = running.get(name); @@ -180,12 +177,8 @@ public class PluginLoader implements LifecycleListener { active.stop(); running.remove(name); - clean = true; } - } - if (clean) { - System.gc(); - processPendingCleanups(); + cleanInBackground(); } } @@ -193,7 +186,7 @@ public class PluginLoader implements LifecycleListener { public synchronized void start() { log.info("Loading plugins from " + pluginsDir.getAbsolutePath()); srvInfoImpl.state = ServerInformation.State.STARTUP; - rescan(false); + rescan(); srvInfoImpl.state = ServerInformation.State.RUNNING; if (scanner != null) { scanner.start(); @@ -207,26 +200,18 @@ public class PluginLoader implements LifecycleListener { } srvInfoImpl.state = ServerInformation.State.SHUTDOWN; synchronized (this) { - boolean clean = !running.isEmpty(); for (Plugin p : running.values()) { p.stop(); } running.clear(); broken.clear(); - if (clean) { + if (cleanupHandles.size() > running.size()) { System.gc(); processPendingCleanups(); } } } - public void rescan(boolean forceCleanup) { - if (rescanImp() || forceCleanup) { - System.gc(); - processPendingCleanups(); - } - } - public void reload(List names) throws InvalidPluginException, PluginInstallException { synchronized (this) { @@ -256,14 +241,14 @@ public class PluginLoader implements LifecycleListener { throw e; } } - System.gc(); - processPendingCleanups(); + + cleanInBackground(); } } - private synchronized boolean rescanImp() { + public synchronized void rescan() { List jars = scanJarsInPluginsDirectory(); - boolean clean = stopRemovedPlugins(jars); + stopRemovedPlugins(jars); for (File jar : jars) { String name = nameOf(jar); @@ -285,14 +270,13 @@ public class PluginLoader implements LifecycleListener { runPlugin(name, jar, active); if (active == null) { log.info(String.format("Loaded plugin %s", name)); - } else { - clean = true; } } catch (PluginInstallException e) { log.warn(String.format("Cannot load plugin %s", name), e.getCause()); } } - return clean; + + cleanInBackground(); } private void runPlugin(String name, File jar, Plugin oldPlugin) @@ -322,7 +306,7 @@ public class PluginLoader implements LifecycleListener { } } - private boolean stopRemovedPlugins(List jars) { + private void stopRemovedPlugins(List jars) { Set unload = Sets.newHashSet(running.keySet()); for (File jar : jars) { unload.remove(nameOf(jar)); @@ -331,15 +315,22 @@ public class PluginLoader implements LifecycleListener { log.info(String.format("Unloading plugin %s", name)); running.remove(name).stop(); } - return !unload.isEmpty(); } - private synchronized void processPendingCleanups() { + synchronized int processPendingCleanups() { CleanupHandle h; while ((h = (CleanupHandle) cleanupQueue.poll()) != null) { h.cleanup(); cleanupHandles.remove(h); } + return Math.max(0, cleanupHandles.size() - running.size()); + } + + private void cleanInBackground() { + int cnt = Math.max(0, cleanupHandles.size() - running.size()); + if (0 < cnt) { + cleaner.get().clean(cnt); + } } private static String nameOf(File jar) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java index 5f5b8e3633..ab7dc3c839 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java @@ -23,8 +23,10 @@ public class PluginModule extends LifecycleModule { bind(ServerInformationImpl.class); bind(ServerInformation.class).to(ServerInformationImpl.class); + bind(PluginCleanerTask.class); bind(PluginGuiceEnvironment.class); bind(PluginLoader.class); + bind(CopyConfigModule.class); listener().to(PluginLoader.class); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java index b2e3fedeb3..a484c5d44b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java @@ -38,7 +38,7 @@ class PluginScannerThread extends Thread { } } catch (InterruptedException e) { } - loader.rescan(false); + loader.rescan(); } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java index e01c6d8dd0..d60465cca2 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java @@ -37,7 +37,7 @@ final class PluginReloadCommand extends SshCommand { @Override protected void run() throws UnloggedFailure { if (names == null || names.isEmpty()) { - loader.rescan(true); + loader.rescan(); } else { try { loader.reload(names);