Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
Elasticsearch: Base the default number of shards on ES version
Disallow change index task duplication
AbstractPushForReview: Add tests for pushing with skip-validation option
Add extension point to gr-user-header
Change-Id: I6de8e61e110046d645c6da0bb71923d491a5764a
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 7d7f64b..78100ae 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3055,7 +3055,7 @@
link:https://www.elastic.co/guide/en/elasticsearch/reference/current/getting-started-concepts.html#getting-started-shards-and-replicas[
Elasticsearch documentation] for details.
+
-Defaults to 5.
+Defaults to 5 for Elasticsearch versions 5 and 6, and to 1 starting with Elasticsearch 7.
[[elasticsearch.numberOfReplicas]]elasticsearch.numberOfReplicas::
+
diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index 8f18b61..d206e88 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -187,7 +187,7 @@
}
// Recreate the index.
- String indexCreationFields = concatJsonString(getSettings(), getMappings());
+ String indexCreationFields = concatJsonString(getSettings(client.adapter()), getMappings());
response =
performRequest(
"PUT", indexName + client.adapter().includeTypeNameParam(), indexCreationFields);
@@ -202,8 +202,8 @@
protected abstract String getMappings();
- private String getSettings() {
- return gson.toJson(ImmutableMap.of(SETTINGS, ElasticSetting.createSetting(config)));
+ private String getSettings(ElasticQueryAdapter adapter) {
+ return gson.toJson(ImmutableMap.of(SETTINGS, ElasticSetting.createSetting(config, adapter)));
}
protected abstract String getId(V v);
diff --git a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
index 5f48499..cbe9bc7 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
@@ -42,7 +42,7 @@
static final String KEY_NUMBER_OF_REPLICAS = "numberOfReplicas";
static final String DEFAULT_PORT = "9200";
static final String DEFAULT_USERNAME = "elastic";
- static final int DEFAULT_NUMBER_OF_SHARDS = 5;
+ static final int DEFAULT_NUMBER_OF_SHARDS = 0;
static final int DEFAULT_NUMBER_OF_REPLICAS = 1;
private final Config cfg;
@@ -100,4 +100,11 @@
String getIndexName(String name, int schemaVersion) {
return String.format("%s%s_%04d", prefix, name, schemaVersion);
}
+
+ int getNumberOfShards(ElasticQueryAdapter adapter) {
+ if (numberOfShards == DEFAULT_NUMBER_OF_SHARDS) {
+ return adapter.getDefaultNumberOfShards();
+ }
+ return numberOfShards;
+ }
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
index b015678..72c52b0 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
@@ -27,6 +27,7 @@
private final boolean useV5Type;
private final boolean useV6Type;
private final boolean omitType;
+ private final int defaultNumberOfShards;
private final String searchFilteringName;
private final String indicesExistParams;
@@ -41,6 +42,7 @@
this.useV5Type = !version.isV6OrLater();
this.useV6Type = version.isV6();
this.omitType = version.isV7OrLater();
+ this.defaultNumberOfShards = version.isV7OrLater() ? 1 : 5;
this.versionDiscoveryUrl = version.isV6OrLater() ? "/%s*" : "/%s*/_aliases";
this.searchFilteringName = "_source";
this.indicesExistParams =
@@ -98,6 +100,10 @@
return omitType;
}
+ int getDefaultNumberOfShards() {
+ return defaultNumberOfShards;
+ }
+
String getType() {
return getType("");
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticSetting.java b/java/com/google/gerrit/elasticsearch/ElasticSetting.java
index 98c313c..14e4623 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticSetting.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticSetting.java
@@ -22,18 +22,18 @@
private static final ImmutableMap<String, String> CUSTOM_CHAR_MAPPING =
ImmutableMap.of("\\u002E", "\\u0020", "\\u005F", "\\u0020");
- static SettingProperties createSetting(ElasticConfiguration config) {
- return new ElasticSetting.Builder().addCharFilter().addAnalyzer().build(config);
+ static SettingProperties createSetting(ElasticConfiguration config, ElasticQueryAdapter adapter) {
+ return new ElasticSetting.Builder().addCharFilter().addAnalyzer().build(config, adapter);
}
static class Builder {
private final ImmutableMap.Builder<String, FieldProperties> fields =
new ImmutableMap.Builder<>();
- SettingProperties build(ElasticConfiguration config) {
+ SettingProperties build(ElasticConfiguration config, ElasticQueryAdapter adapter) {
SettingProperties properties = new SettingProperties();
properties.analysis = fields.build();
- properties.numberOfShards = config.numberOfShards;
+ properties.numberOfShards = config.getNumberOfShards(adapter);
properties.numberOfReplicas = config.numberOfReplicas;
return properties;
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 064af64..5e75c48 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
+import com.google.common.base.Objects;
import com.google.common.flogger.FluentLogger;
import com.google.common.util.concurrent.Atomics;
import com.google.common.util.concurrent.Futures;
@@ -53,7 +54,9 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -97,6 +100,11 @@
private final StalenessChecker stalenessChecker;
private final boolean autoReindexIfStale;
+ private final Set<IndexTask> queuedIndexTasks =
+ Collections.newSetFromMap(new ConcurrentHashMap<>());
+ private final Set<ReindexIfStaleTask> queuedReindexIfStaleTasks =
+ Collections.newSetFromMap(new ConcurrentHashMap<>());
+
@AssistedInject
ChangeIndexer(
@GerritServerConfig Config cfg,
@@ -164,7 +172,11 @@
@SuppressWarnings("deprecation")
public com.google.common.util.concurrent.CheckedFuture<?, IOException> indexAsync(
Project.NameKey project, Change.Id id) {
- return submit(new IndexTask(project, id));
+ IndexTask task = new IndexTask(project, id);
+ if (queuedIndexTasks.add(task)) {
+ return submit(task);
+ }
+ return Futures.immediateCheckedFuture(null);
}
/**
@@ -288,7 +300,11 @@
@SuppressWarnings("deprecation")
public com.google.common.util.concurrent.CheckedFuture<Boolean, IOException> reindexIfStale(
Project.NameKey project, Change.Id id) {
- return submit(new ReindexIfStaleTask(project, id), batchExecutor);
+ ReindexIfStaleTask task = new ReindexIfStaleTask(project, id);
+ if (queuedReindexIfStaleTasks.add(task)) {
+ return submit(task, batchExecutor);
+ }
+ return Futures.immediateCheckedFuture(false);
}
private void autoReindexIfStale(ChangeData cd) {
@@ -331,6 +347,8 @@
protected abstract T callImpl(Provider<ReviewDb> db) throws Exception;
+ protected abstract void remove();
+
@Override
public abstract String toString();
@@ -383,15 +401,35 @@
@Override
public Void callImpl(Provider<ReviewDb> db) throws Exception {
+ remove();
ChangeData cd = newChangeData(db.get(), project, id);
index(cd);
return null;
}
@Override
+ public int hashCode() {
+ return Objects.hashCode(IndexTask.class, id.get());
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (!(obj instanceof IndexTask)) {
+ return false;
+ }
+ IndexTask other = (IndexTask) obj;
+ return id.get() == other.id.get();
+ }
+
+ @Override
public String toString() {
return "index-change-" + id;
}
+
+ @Override
+ protected void remove() {
+ queuedIndexTasks.remove(this);
+ }
}
// Not AbstractIndexTask as it doesn't need ReviewDb.
@@ -427,6 +465,7 @@
@Override
public Boolean callImpl(Provider<ReviewDb> db) throws Exception {
+ remove();
try {
if (stalenessChecker.isStale(id)) {
indexImpl(newChangeData(db.get(), project, id));
@@ -446,9 +485,28 @@
}
@Override
+ public int hashCode() {
+ return Objects.hashCode(ReindexIfStaleTask.class, id.get());
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (!(obj instanceof ReindexIfStaleTask)) {
+ return false;
+ }
+ ReindexIfStaleTask other = (ReindexIfStaleTask) obj;
+ return id.get() == other.id.get();
+ }
+
+ @Override
public String toString() {
return "reindex-if-stale-change-" + id;
}
+
+ @Override
+ protected void remove() {
+ queuedReindexIfStaleTasks.remove(this);
+ }
}
private boolean isCausedByRepositoryNotFoundException(Throwable throwable) {
diff --git a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
index 609432b..be4f38c 100644
--- a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
+++ b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
@@ -17,6 +17,7 @@
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
+import com.google.common.base.Objects;
import com.google.common.flogger.FluentLogger;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
@@ -44,8 +45,11 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.util.Collections;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future;
import org.eclipse.jgit.lib.Config;
@@ -63,6 +67,8 @@
private final ListeningExecutorService executor;
private final boolean enabled;
+ private final Set<Index> queuedIndexTasks = Collections.newSetFromMap(new ConcurrentHashMap<>());
+
@Inject
ReindexAfterRefUpdate(
@GerritServerConfig Config cfg,
@@ -113,9 +119,12 @@
@Override
public void onSuccess(List<Change> changes) {
for (Change c : changes) {
- // Don't retry indefinitely; if this fails changes may be stale.
- @SuppressWarnings("unused")
- Future<?> possiblyIgnoredError = executor.submit(new Index(event, c.getId()));
+ Index task = new Index(event, c.getId());
+ if (queuedIndexTasks.add(task)) {
+ // Don't retry indefinitely; if this fails changes may be stale.
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = executor.submit(task);
+ }
}
}
@@ -145,6 +154,8 @@
}
protected abstract V impl(RequestContext ctx) throws Exception;
+
+ protected abstract void remove();
}
private class GetChanges extends Task<List<Change>> {
@@ -169,6 +180,9 @@
+ " update of project "
+ event.getProjectName();
}
+
+ @Override
+ protected void remove() {}
}
private class Index extends Task<Void> {
@@ -183,6 +197,7 @@
protected Void impl(RequestContext ctx) throws OrmException, IOException {
// Reload change, as some time may have passed since GetChanges.
ReviewDb db = ctx.getReviewDbProvider().get();
+ remove();
try {
Change c =
notesFactory
@@ -196,8 +211,27 @@
}
@Override
+ public int hashCode() {
+ return Objects.hashCode(Index.class, id.get());
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (!(obj instanceof Index)) {
+ return false;
+ }
+ Index other = (Index) obj;
+ return id.get() == other.id.get();
+ }
+
+ @Override
public String toString() {
return "Index change " + id.get() + " of project " + event.getProjectName();
}
+
+ @Override
+ protected void remove() {
+ queuedIndexTasks.remove(this);
+ }
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 1226661..0e663f3 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -77,6 +77,8 @@
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.testing.EditInfoSubject;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
@@ -87,16 +89,22 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.receive.NoteDbPushOption;
import com.google.gerrit.server.git.receive.ReceiveConstants;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.git.validators.CommitValidators.ChangeIdValidator;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.testing.Util;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gerrit.testing.TestTimeUtil;
+import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashMap;
@@ -104,6 +112,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.eclipse.jgit.api.errors.GitAPIException;
@@ -133,6 +142,8 @@
private LabelType patchSetLock;
+ @Inject private DynamicSet<CommitValidationListener> commitValidators;
+
@BeforeClass
public static void setTimeForTesting() {
TestTimeUtil.resetWithClockStep(1, SECONDS);
@@ -2255,6 +2266,57 @@
.isEqualTo(Iterables.getLast(commits).name());
}
+ private static class TestValidator implements CommitValidationListener {
+ private final AtomicInteger count = new AtomicInteger();
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ count.incrementAndGet();
+ return Collections.emptyList();
+ }
+
+ public int count() {
+ return count.get();
+ }
+ }
+
+ @Test
+ public void skipValidation() throws Exception {
+ String master = "refs/heads/master";
+ TestValidator validator = new TestValidator();
+ RegistrationHandle handle = commitValidators.add("test-validator", validator);
+
+ try {
+ // Validation listener is called on normal push
+ PushOneCommit push =
+ pushFactory.create(db, admin.getIdent(), testRepo, "change1", "a.txt", "content");
+ PushOneCommit.Result r = push.to(master);
+ r.assertOkStatus();
+ assertThat(validator.count()).isEqualTo(1);
+
+ // Push is rejected and validation listener is not called when not allowed
+ // to use skip option
+ PushOneCommit push2 =
+ pushFactory.create(db, admin.getIdent(), testRepo, "change2", "b.txt", "content");
+ push2.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ r = push2.to(master);
+ r.assertErrorStatus("not permitted: skip validation");
+ assertThat(validator.count()).isEqualTo(1);
+
+ // Validation listener is not called when skip option is used
+ grantSkipValidation(project, master, SystemGroupBackend.REGISTERED_USERS);
+ PushOneCommit push3 =
+ pushFactory.create(db, admin.getIdent(), testRepo, "change2", "b.txt", "content");
+ push3.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ r = push3.to(master);
+ r.assertOkStatus();
+ assertThat(validator.count()).isEqualTo(1);
+ } finally {
+ handle.remove();
+ }
+ }
+
@Test
public void pushToPublishMagicBranchIsAllowed() throws Exception {
// Push to "refs/publish/*" will be a synonym of "refs/for/*".
diff --git a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.html b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.html
index 89e2b7d..6accdfc 100644
--- a/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.html
+++ b/polygerrit-ui/app/elements/change-list/gr-user-header/gr-user-header.html
@@ -16,12 +16,14 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
+<link rel="import" href="../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.html">
+<link rel="import" href="../../plugins/gr-endpoint-param/gr-endpoint-param.html">
<link rel="import" href="../../shared/gr-avatar/gr-avatar.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/dashboard-header-styles.html">
-<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-user-header">
<template>
@@ -62,6 +64,12 @@
date-str="[[_computeDetail(_accountDetails, 'registered_on')]]">
</gr-date-formatter>
</div>
+ <gr-endpoint-decorator name="user-header">
+ <gr-endpoint-param name="accountDetails" value="[[_accountDetails]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="loggedIn" value="[[loggedIn]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
</div>
<div class="info">
<div class$="[[_computeDashboardLinkClass(showDashboardLink, loggedIn)]]">
