opendev/gerrit
Code Issues Proposed changes

Merge changes from topic "issue-7611" into stable-2.15

Browse Source 
* changes:
  Properly await termination of all index executor threads on shutdown
  Stop ProjectCacheClock thread on server shutdown
  CleanUp after query tests
This commit is contained in:
David Pursehouse 2017-11-04 00:22:49 +00:00 committed by Gerrit Code Review
commit 305dccfe2d
9 changed files with 88 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-index/src/main/java/com/google/gerrit/index/Index.java
View File

@ -36,9 +36,6 @@ public interface Index<K, V> {
/** @return the schema version used by this index. */
Schema<V> getSchema();
/** Stop and await termination of all executor threads */
default void stop() {}
/** Close this index. */
void close();

1
gerrit-index/src/main/java/com/google/gerrit/index/IndexCollection.java
View File

@ -95,7 +95,6 @@ public abstract class IndexCollection<K, V, I extends Index<K, V>> implements Li
}
for (I write : writeIndexes) {
if (write != read) {
write.stop();
write.close();
}
}

7
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@ -35,7 +35,6 @@ import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.Predicate;
@ -74,7 +73,6 @@ import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.apache.lucene.document.Document;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexableField;
@ -187,11 +185,6 @@ public class LuceneChangeIndex implements ChangeIndex {
}
}
@Override
public void stop() {
MoreExecutors.shutdownAndAwaitTermination(executor, Long.MAX_VALUE, TimeUnit.SECONDS);
}
@Override
public void close() {
try {

41
gerrit-server/src/main/java/com/google/gerrit/server/index/IndexModule.java
View File

@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.SchemaDefinitions;
@ -45,6 +46,7 @@ import com.google.gerrit.server.index.group.GroupIndexRewriter;
import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.index.group.GroupIndexerImpl;
import com.google.gerrit.server.index.group.GroupSchemaDefinitions;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Key;
import com.google.inject.Provides;
@ -52,6 +54,7 @@ import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import java.util.Collection;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Config;
/**
@ -81,11 +84,13 @@ public class IndexModule extends LifecycleModule {
private final int threads;
private final ListeningExecutorService interactiveExecutor;
private final ListeningExecutorService batchExecutor;
private final boolean closeExecutorsOnShutdown;
public IndexModule(int threads) {
this.threads = threads;
this.interactiveExecutor = null;
this.batchExecutor = null;
this.closeExecutorsOnShutdown = true;
}
public IndexModule(
@ -93,6 +98,7 @@ public class IndexModule extends LifecycleModule {
this.threads = -1;
this.interactiveExecutor = interactiveExecutor;
this.batchExecutor = batchExecutor;
this.closeExecutorsOnShutdown = false;
}
@Override
@ -112,6 +118,17 @@ public class IndexModule extends LifecycleModule {
listener().to(GroupIndexCollection.class);
factory(GroupIndexerImpl.Factory.class);
if (closeExecutorsOnShutdown) {
// The executors must be shutdown _before_ closing the indexes.
// On Gerrit start the LifecycleListeners are invoked in the order in which they are
// registered, but on shutdown of Gerrit the order is reversed. This means the
// LifecycleListener to shutdown the executors must be registered _after_ the
// LifecycleListeners that close the indexes. The closing of the indexes is done by
// *IndexCollection which have been registered as LifecycleListener above. The
// registration of the ShutdownIndexExecutors LifecycleListener must happen afterwards.
listener().to(ShutdownIndexExecutors.class);
}
DynamicSet.setOf(binder(), OnlineUpgradeListener.class);
}
@ -186,4 +203,28 @@ public class IndexModule extends LifecycleModule {
}
return MoreExecutors.listeningDecorator(workQueue.createQueue(threads, "Index-Batch"));
}
@Singleton
private static class ShutdownIndexExecutors implements LifecycleListener {
private final ListeningExecutorService interactiveExecutor;
private final ListeningExecutorService batchExecutor;
@Inject
ShutdownIndexExecutors(
@IndexExecutor(INTERACTIVE) ListeningExecutorService interactiveExecutor,
@IndexExecutor(BATCH) ListeningExecutorService batchExecutor) {
this.interactiveExecutor = interactiveExecutor;
this.batchExecutor = batchExecutor;
}
@Override
public void start() {}
@Override
public void stop() {
MoreExecutors.shutdownAndAwaitTermination(
interactiveExecutor, Long.MAX_VALUE, TimeUnit.SECONDS);
MoreExecutors.shutdownAndAwaitTermination(batchExecutor, Long.MAX_VALUE, TimeUnit.SECONDS);
}
}
}

23
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheClock.java
View File

@ -15,6 +15,7 @@
package com.google.gerrit.server.project;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
@ -28,15 +29,22 @@ import org.eclipse.jgit.lib.Config;
/** Ticks periodically to force refresh events for {@link ProjectCacheImpl}. */
@Singleton
public class ProjectCacheClock {
public class ProjectCacheClock implements LifecycleListener {
private final Config serverConfig;
private final AtomicLong generation = new AtomicLong();
private ScheduledExecutorService executor;
@Inject
public ProjectCacheClock(@GerritServerConfig Config serverConfig) {
this(checkFrequency(serverConfig));
this.serverConfig = serverConfig;
}
public ProjectCacheClock(long checkFrequencyMillis) {
@Override
public void start() {
long checkFrequencyMillis = checkFrequency(serverConfig);
if (checkFrequencyMillis == Long.MAX_VALUE) {
// Start with generation 1 (to avoid magic 0 below).
// Do not begin background thread, disabling the clock.
@ -44,7 +52,7 @@ public class ProjectCacheClock {
} else if (10 < checkFrequencyMillis) {
// Start with generation 1 (to avoid magic 0 below).
generation.set(1);
ScheduledExecutorService executor =
executor =
Executors.newScheduledThreadPool(
1,
new ThreadFactoryBuilder()
@ -68,6 +76,13 @@ public class ProjectCacheClock {
}
}
@Override
public void stop() {
if (executor != null) {
executor.shutdown();
}
}
long read() {
return generation.get();
}

15
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
View File

@ -20,7 +20,7 @@ import com.google.common.base.Throwables;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.Sets;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.cache.CacheModule;
@ -32,7 +32,6 @@ import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.internal.UniqueAnnotations;
import com.google.inject.name.Named;
import java.io.IOException;
import java.util.Collections;
@ -69,9 +68,15 @@ public class ProjectCacheImpl implements ProjectCache {
bind(ProjectCacheImpl.class);
bind(ProjectCache.class).to(ProjectCacheImpl.class);
bind(LifecycleListener.class)
.annotatedWith(UniqueAnnotations.create())
.to(ProjectCacheWarmer.class);
install(
new LifecycleModule() {
@Override
protected void configure() {
listener().to(ProjectCacheWarmer.class);
listener().to(ProjectCacheClock.class);
}
});
}
};
}

6
gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
View File

@ -129,6 +129,12 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
setUpDatabase();
}
@After
public void cleanUp() {
lifecycle.stop();
db.close();
}
protected void setUpDatabase() throws Exception {
db = schemaFactory.open();
schemaCreator.create(db);

6
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@ -204,6 +204,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
setUpDatabase();
}
@After
public void cleanUp() {
lifecycle.stop();
db.close();
}
protected void setUpDatabase() throws Exception {
try (ReviewDb underlyingDb = inMemoryDatabase.getDatabase().open()) {
schemaCreator.create(underlyingDb);

6
gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
View File

@ -126,6 +126,12 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
setUpDatabase();
}
@After
public void cleanUp() {
lifecycle.stop();
db.close();
}
protected void setUpDatabase() throws Exception {
db = schemaFactory.open();
schemaCreator.create(db);