Split mergeability checks by priority
When a project config is updated or a change is merged, all open changes on an entire project or branch need to be rechecked for mergeability. For projects or branches with lots of open changes, this can flood the queue with potentially slow checks. This will block simpler cases, such as checking mergeability of a single change during ReceiveCommits. Define a new Priority enum on the MergeabilityChecksExecutor so we can split the queue based on priority, allowing smaller, interactive changes such as pushes to not wait for the entire queued backlog. Change-Id: I57f9eb33dbbcd8a560adfe4458ce48d7b0817957
This commit is contained in:
parent
b49726e145
commit
6ea964a0a1
@ -823,6 +823,15 @@ changes is updated.
|
|||||||
+
|
+
|
||||||
Default is 1.
|
Default is 1.
|
||||||
|
|
||||||
|
[[changeMerge.interactiveThreadPoolSize]]changeMerge.interactiveThreadPoolSize::
|
||||||
|
+
|
||||||
|
Maximum size of the thread pool in which the mergeability flag of open
|
||||||
|
changes is updated, when processing interactive user requests (e.g.
|
||||||
|
pushes to refs/for/*). Set to 0 or negative to share the pool for
|
||||||
|
background mergeability checks.
|
||||||
|
+
|
||||||
|
Default is 1.
|
||||||
|
|
||||||
[[commentlink]]
|
[[commentlink]]
|
||||||
=== Section commentlink
|
=== Section commentlink
|
||||||
|
|
||||||
|
@ -44,6 +44,7 @@ import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
|
|||||||
import com.google.gerrit.server.change.ChangeKindCache;
|
import com.google.gerrit.server.change.ChangeKindCache;
|
||||||
import com.google.gerrit.server.change.MergeabilityChecker;
|
import com.google.gerrit.server.change.MergeabilityChecker;
|
||||||
import com.google.gerrit.server.change.MergeabilityChecksExecutor;
|
import com.google.gerrit.server.change.MergeabilityChecksExecutor;
|
||||||
|
import com.google.gerrit.server.change.MergeabilityChecksExecutor.Priority;
|
||||||
import com.google.gerrit.server.change.PatchSetInserter;
|
import com.google.gerrit.server.change.PatchSetInserter;
|
||||||
import com.google.gerrit.server.config.CanonicalWebUrl;
|
import com.google.gerrit.server.config.CanonicalWebUrl;
|
||||||
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
|
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
|
||||||
@ -290,11 +291,20 @@ public class Reindex extends SiteProgram {
|
|||||||
|
|
||||||
@Provides
|
@Provides
|
||||||
@Singleton
|
@Singleton
|
||||||
@MergeabilityChecksExecutor
|
@MergeabilityChecksExecutor(Priority.BACKGROUND)
|
||||||
public WorkQueue.Executor createMergeabilityChecksExecutor(
|
public WorkQueue.Executor createMergeabilityChecksExecutor(
|
||||||
WorkQueue queues) {
|
WorkQueue queues) {
|
||||||
return queues.createQueue(1, "MergeabilityChecks");
|
return queues.createQueue(1, "MergeabilityChecks");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Provides
|
||||||
|
@Singleton
|
||||||
|
@MergeabilityChecksExecutor(Priority.INTERACTIVE)
|
||||||
|
public WorkQueue.Executor createInteractiveMergeabilityChecksExecutor(
|
||||||
|
@MergeabilityChecksExecutor(Priority.BACKGROUND)
|
||||||
|
WorkQueue.Executor bg) {
|
||||||
|
return bg;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private int indexAll() throws Exception {
|
private int indexAll() throws Exception {
|
||||||
|
@ -34,6 +34,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
|
|||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
import com.google.gerrit.server.CurrentUser;
|
import com.google.gerrit.server.CurrentUser;
|
||||||
import com.google.gerrit.server.IdentifiedUser;
|
import com.google.gerrit.server.IdentifiedUser;
|
||||||
|
import com.google.gerrit.server.change.MergeabilityChecksExecutor.Priority;
|
||||||
import com.google.gerrit.server.change.Mergeable.MergeableInfo;
|
import com.google.gerrit.server.change.Mergeable.MergeableInfo;
|
||||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||||
import com.google.gerrit.server.git.ProjectConfig;
|
import com.google.gerrit.server.git.ProjectConfig;
|
||||||
@ -85,11 +86,13 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
|
|||||||
private List<Project.NameKey> projects;
|
private List<Project.NameKey> projects;
|
||||||
private boolean force;
|
private boolean force;
|
||||||
private boolean reindex;
|
private boolean reindex;
|
||||||
|
private boolean interactive;
|
||||||
|
|
||||||
private Check() {
|
private Check() {
|
||||||
changes = Lists.newArrayListWithExpectedSize(1);
|
changes = Lists.newArrayListWithExpectedSize(1);
|
||||||
branches = Lists.newArrayListWithExpectedSize(1);
|
branches = Lists.newArrayListWithExpectedSize(1);
|
||||||
projects = Lists.newArrayListWithExpectedSize(1);
|
projects = Lists.newArrayListWithExpectedSize(1);
|
||||||
|
interactive = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
public Check addChange(Change change) {
|
public Check addChange(Change change) {
|
||||||
@ -99,11 +102,13 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
|
|||||||
|
|
||||||
public Check addBranch(Branch.NameKey branch) {
|
public Check addBranch(Branch.NameKey branch) {
|
||||||
branches.add(branch);
|
branches.add(branch);
|
||||||
|
interactive = false;
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
public Check addProject(Project.NameKey project) {
|
public Check addProject(Project.NameKey project) {
|
||||||
projects.add(project);
|
projects.add(project);
|
||||||
|
interactive = false;
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -119,7 +124,12 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
|
|||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private ListeningExecutorService getExecutor() {
|
||||||
|
return interactive ? interactiveExecutor : backgroundExecutor;
|
||||||
|
}
|
||||||
|
|
||||||
public CheckedFuture<?, IOException> runAsync() {
|
public CheckedFuture<?, IOException> runAsync() {
|
||||||
|
final ListeningExecutorService executor = getExecutor();
|
||||||
ListenableFuture<List<Change>> getChanges;
|
ListenableFuture<List<Change>> getChanges;
|
||||||
if (branches.isEmpty() && projects.isEmpty()) {
|
if (branches.isEmpty() && projects.isEmpty()) {
|
||||||
getChanges = Futures.immediateFuture(changes);
|
getChanges = Futures.immediateFuture(changes);
|
||||||
@ -201,7 +211,8 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
|
|||||||
private final ChangeControl.GenericFactory changeControlFactory;
|
private final ChangeControl.GenericFactory changeControlFactory;
|
||||||
private final Provider<Mergeable> mergeable;
|
private final Provider<Mergeable> mergeable;
|
||||||
private final ChangeIndexer indexer;
|
private final ChangeIndexer indexer;
|
||||||
private final ListeningExecutorService executor;
|
private final ListeningExecutorService backgroundExecutor;
|
||||||
|
private final ListeningExecutorService interactiveExecutor;
|
||||||
private final MergeabilityCheckQueue mergeabilityCheckQueue;
|
private final MergeabilityCheckQueue mergeabilityCheckQueue;
|
||||||
private final MetaDataUpdate.Server metaDataUpdateFactory;
|
private final MetaDataUpdate.Server metaDataUpdateFactory;
|
||||||
|
|
||||||
@ -211,7 +222,10 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
|
|||||||
IdentifiedUser.GenericFactory identifiedUserFactory,
|
IdentifiedUser.GenericFactory identifiedUserFactory,
|
||||||
ChangeControl.GenericFactory changeControlFactory,
|
ChangeControl.GenericFactory changeControlFactory,
|
||||||
Provider<Mergeable> mergeable, ChangeIndexer indexer,
|
Provider<Mergeable> mergeable, ChangeIndexer indexer,
|
||||||
@MergeabilityChecksExecutor Executor executor,
|
@MergeabilityChecksExecutor(Priority.BACKGROUND)
|
||||||
|
Executor backgroundExecutor,
|
||||||
|
@MergeabilityChecksExecutor(Priority.INTERACTIVE)
|
||||||
|
Executor interactiveExecutor,
|
||||||
MergeabilityCheckQueue mergeabilityCheckQueue,
|
MergeabilityCheckQueue mergeabilityCheckQueue,
|
||||||
MetaDataUpdate.Server metaDataUpdateFactory) {
|
MetaDataUpdate.Server metaDataUpdateFactory) {
|
||||||
this.tl = tl;
|
this.tl = tl;
|
||||||
@ -220,7 +234,10 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
|
|||||||
this.changeControlFactory = changeControlFactory;
|
this.changeControlFactory = changeControlFactory;
|
||||||
this.mergeable = mergeable;
|
this.mergeable = mergeable;
|
||||||
this.indexer = indexer;
|
this.indexer = indexer;
|
||||||
this.executor = MoreExecutors.listeningDecorator(executor);
|
this.backgroundExecutor =
|
||||||
|
MoreExecutors.listeningDecorator(backgroundExecutor);
|
||||||
|
this.interactiveExecutor =
|
||||||
|
MoreExecutors.listeningDecorator(interactiveExecutor);
|
||||||
this.mergeabilityCheckQueue = mergeabilityCheckQueue;
|
this.mergeabilityCheckQueue = mergeabilityCheckQueue;
|
||||||
this.metaDataUpdateFactory = metaDataUpdateFactory;
|
this.metaDataUpdateFactory = metaDataUpdateFactory;
|
||||||
}
|
}
|
||||||
|
@ -28,4 +28,9 @@ import java.lang.annotation.Retention;
|
|||||||
@Retention(RUNTIME)
|
@Retention(RUNTIME)
|
||||||
@BindingAnnotation
|
@BindingAnnotation
|
||||||
public @interface MergeabilityChecksExecutor {
|
public @interface MergeabilityChecksExecutor {
|
||||||
|
public enum Priority {
|
||||||
|
BACKGROUND, INTERACTIVE;
|
||||||
|
}
|
||||||
|
|
||||||
|
Priority value();
|
||||||
}
|
}
|
||||||
|
@ -14,6 +14,7 @@
|
|||||||
|
|
||||||
package com.google.gerrit.server.change;
|
package com.google.gerrit.server.change;
|
||||||
|
|
||||||
|
import com.google.gerrit.server.change.MergeabilityChecksExecutor.Priority;
|
||||||
import com.google.gerrit.server.config.GerritServerConfig;
|
import com.google.gerrit.server.config.GerritServerConfig;
|
||||||
import com.google.gerrit.server.git.WorkQueue;
|
import com.google.gerrit.server.git.WorkQueue;
|
||||||
import com.google.inject.AbstractModule;
|
import com.google.inject.AbstractModule;
|
||||||
@ -22,7 +23,6 @@ import com.google.inject.Singleton;
|
|||||||
|
|
||||||
import org.eclipse.jgit.lib.Config;
|
import org.eclipse.jgit.lib.Config;
|
||||||
|
|
||||||
|
|
||||||
/** Module providing the {@link MergeabilityChecksExecutor}. */
|
/** Module providing the {@link MergeabilityChecksExecutor}. */
|
||||||
public class MergeabilityChecksExecutorModule extends AbstractModule {
|
public class MergeabilityChecksExecutorModule extends AbstractModule {
|
||||||
@Override
|
@Override
|
||||||
@ -31,11 +31,27 @@ public class MergeabilityChecksExecutorModule extends AbstractModule {
|
|||||||
|
|
||||||
@Provides
|
@Provides
|
||||||
@Singleton
|
@Singleton
|
||||||
@MergeabilityChecksExecutor
|
@MergeabilityChecksExecutor(Priority.BACKGROUND)
|
||||||
public WorkQueue.Executor createMergeabilityChecksExecutor(
|
public WorkQueue.Executor createMergeabilityChecksExecutor(
|
||||||
@GerritServerConfig Config config,
|
@GerritServerConfig Config config,
|
||||||
WorkQueue queues) {
|
WorkQueue queues) {
|
||||||
int poolSize = config.getInt("changeMerge", null, "threadPoolSize", 1);
|
int poolSize = config.getInt("changeMerge", null, "threadPoolSize", 1);
|
||||||
return queues.createQueue(poolSize, "MergeabilityChecks");
|
return queues.createQueue(poolSize, "MergeabilityChecks");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Provides
|
||||||
|
@Singleton
|
||||||
|
@MergeabilityChecksExecutor(Priority.INTERACTIVE)
|
||||||
|
public WorkQueue.Executor createMergeabilityChecksExecutor(
|
||||||
|
@GerritServerConfig Config config,
|
||||||
|
WorkQueue queues,
|
||||||
|
@MergeabilityChecksExecutor(Priority.BACKGROUND)
|
||||||
|
WorkQueue.Executor backgroundExecutor) {
|
||||||
|
int poolSize =
|
||||||
|
config.getInt("changeMerge", null, "interactiveThreadPoolSize", 1);
|
||||||
|
if (poolSize <= 0) {
|
||||||
|
return backgroundExecutor;
|
||||||
|
}
|
||||||
|
return queues.createQueue(poolSize, "InteractiveMergeabilityChecks");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user