Add Gerrit config to disable mergeability bit in ChangeInfo

Computing mergeability of open changes becomes very expensive for hosts
that have fast-moving branches and a lot of open changes. This commit
scales down this problem by adding a config to remove the bit from
ChangeInfo.

We will consult the mailing list to see if we can remove this bit from
ChangeInfo and the change index completely or add a new MERGEABLE list
option.

Change-Id: I28e90e17abaf369d03ff9d3c0d491903ecf30bf9
This commit is contained in:
Patrick Hiesel
2019-01-24 10:11:23 +01:00
parent 3307aed7b1
commit db8df4886c
5 changed files with 32 additions and 2 deletions

View File

@@ -1214,6 +1214,15 @@ performance improvements by reducing the number of refs in the repo.
+ +
Default is true. Default is true.
[[change.api.excludeMergeableInChangeInfo]]change.api.excludeMergeableInChangeInfo::
+
If true, the mergeability bit in
link:rest-api-changes.html#change-info[ChangeInfo] will never be set. It can
be requested separately through the
link:rest-api-changes.html#get-mergeable[get-mergeable] endpoint.
+
Default is false.
[[change.showAssigneeInChangesTable]]change.showAssigneeInChangesTable:: [[change.showAssigneeInChangesTable]]change.showAssigneeInChangesTable::
+ +
Show assignee field in changes table. If set to false, assignees will Show assignee field in changes table. If set to false, assignees will

View File

@@ -309,6 +309,10 @@ default. Optional fields are:
* `SKIP_MERGEABLE`: skip the `mergeable` field in * `SKIP_MERGEABLE`: skip the `mergeable` field in
link:#change-info[ChangeInfo]. For fast moving projects, this field must link:#change-info[ChangeInfo]. For fast moving projects, this field must
be recomputed often, which is slow for projects with big trees. be recomputed often, which is slow for projects with big trees.
+
This option is deprecated. In a future release, `mergeable` will not be
populated in link:#change-info[ChangeInfo]. It can be requested separately
by calling the link:#get-mergeable[get-mergeable] endpoint.
-- --
[[submittable]] [[submittable]]
@@ -5765,7 +5769,9 @@ Not set for merged changes.
|`mergeable` |optional| |`mergeable` |optional|
Whether the change is mergeable. + Whether the change is mergeable. +
Not set for merged changes, if the change has not yet been tested, or Not set for merged changes, if the change has not yet been tested, or
if the link:#skip_mergeable[skip_mergeable] option is set. if the link:#skip_mergeable[skip_mergeable] option is set or when
link:config-gerrit.html#change.api.excludeMergeableInChangeInfo[change.api.excludeMergeableInChangeInfo]
is set.
|`submittable` |optional| |`submittable` |optional|
Whether the change has been approved by the project submit rules. + Whether the change has been approved by the project submit rules. +
Only set if link:#submittable[requested]. Only set if link:#submittable[requested].

View File

@@ -78,6 +78,7 @@ public enum ListChangesOption {
TRACKING_IDS(21), TRACKING_IDS(21),
/** Skip mergeability data */ /** Skip mergeability data */
@Deprecated
SKIP_MERGEABLE(22); SKIP_MERGEABLE(22);
private final int value; private final int value;

View File

@@ -83,6 +83,7 @@ import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountInfoComparator; import com.google.gerrit.server.account.AccountInfoComparator;
import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
@@ -111,6 +112,7 @@ import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.function.Supplier; import java.util.function.Supplier;
import org.eclipse.jgit.lib.Config;
/** /**
* Produces {@link ChangeInfo} (which is serialized to JSON afterwards) from {@link ChangeData}. * Produces {@link ChangeInfo} (which is serialized to JSON afterwards) from {@link ChangeData}.
@@ -216,6 +218,7 @@ public class ChangeJson {
private final Metrics metrics; private final Metrics metrics;
private final RevisionJson revisionJson; private final RevisionJson revisionJson;
private final Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory; private final Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory;
private final boolean excludeMergeableInChangeInfo;
private final boolean lazyLoad; private final boolean lazyLoad;
private AccountLoader accountLoader; private AccountLoader accountLoader;
@@ -236,6 +239,7 @@ public class ChangeJson {
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
Metrics metrics, Metrics metrics,
RevisionJson.Factory revisionJsonFactory, RevisionJson.Factory revisionJsonFactory,
@GerritServerConfig Config cfg,
@Assisted Iterable<ListChangesOption> options, @Assisted Iterable<ListChangesOption> options,
@Assisted Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory) { @Assisted Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory) {
this.userProvider = user; this.userProvider = user;
@@ -252,6 +256,8 @@ public class ChangeJson {
this.metrics = metrics; this.metrics = metrics;
this.revisionJson = revisionJsonFactory.create(options); this.revisionJson = revisionJsonFactory.create(options);
this.options = Sets.immutableEnumSet(options); this.options = Sets.immutableEnumSet(options);
this.excludeMergeableInChangeInfo =
cfg.getBoolean("change", "api", "excludeMergeableInChangeInfo", false);
this.lazyLoad = containsAnyOf(this.options, REQUIRE_LAZY_LOAD); this.lazyLoad = containsAnyOf(this.options, REQUIRE_LAZY_LOAD);
this.pluginDefinedAttributesFactory = pluginDefinedAttributesFactory; this.pluginDefinedAttributesFactory = pluginDefinedAttributesFactory;
@@ -509,7 +515,7 @@ public class ChangeJson {
if (str.isOk()) { if (str.isOk()) {
out.submitType = str.type; out.submitType = str.type;
} }
if (!has(SKIP_MERGEABLE)) { if (!excludeMergeableInChangeInfo && !has(SKIP_MERGEABLE)) {
out.mergeable = cd.isMergeable(); out.mergeable = cd.isMergeable();
} }
if (has(SUBMITTABLE)) { if (has(SUBMITTABLE)) {

View File

@@ -252,6 +252,14 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(c.mergeable).isTrue(); assertThat(c.mergeable).isTrue();
} }
@Test
@GerritConfig(name = "change.api.excludeMergeableInChangeInfo", value = "true")
public void excludeMergeableInChangeInfo() throws Exception {
PushOneCommit.Result r = createChange();
ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
assertThat(c.mergeable).isNull();
}
@Test @Test
public void setPrivateByOwner() throws Exception { public void setPrivateByOwner() throws Exception {
TestRepository<InMemoryRepository> userRepo = cloneProject(project, user); TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);