Merge branch 'stable-3.5' into stable-3.6
* stable-3.5:
Add an option to periodically warm the project_list cache
Improve test coverage for internal change query pagination
prolog_rules cache: only use memoryLimit from cache.projects
Set version to 3.5.4-SNAPSHOT
Set version to 3.5.3
Don't update the last updated timestamp when running copy-approvals
CreateRefControl: Check permissions on source refs
Set version to 3.4.7-SNAPSHOT
Set version to 3.4.6
Fix javadoc summary for Google Java Style
Don't depend on predicate order as they get sorted based on cost
Fix DefaultMemoryCacheFactory to correctly set refreshAfterWrite
Update git submodules
Shortcut CreateRefControl#checkCreateCommit only for push processing
Paginate internal change index queries
Fix index pagination to set 'more_changes' correctly
Fix Lucene change index search-after implementation
Elasticsearch tests: Recreate container for each test suite
Allow different data reservoirs for metrics
Update git submodules
Bump SSHD version to 2.8.0 and update JGit to stable-6.1 tip (d01376106)
Fix AndSource to run #match() of predicate whose cost is least
Bump JGit to latest stable-5.13 (035e0e23f)
Add getter for registered events
Ib56133b39 is reverted during merge because it does not apply to
stable-3.6 where the copy-approval utility does not exist.
Also add suppress errorprone error of ProtectedMembersInFinalClass
on TestMatchablePredicate and update gitiles plugin to 24529d for
allowing the build to pass successfully.
Release-Notes: skip
Change-Id: Ic2af325c6383ef5640fa3c367bd1c458554945fd
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 79409f7..611a8e0 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1188,7 +1188,7 @@
cache `"prolog_rules"`::
+
Caches parsed `rules.pl` contents for each project. This cache uses the same
-size as the `projects` cache, and cannot be configured independently.
+size as the `projects` cache when `cache.prolog_rules.memoryLimit` is not set.
cache `"pure_revert"`::
+
@@ -1309,6 +1309,21 @@
+
Default is the number of CPUs.
+[[cache.project_list.interval]]cache.project_list.interval::
++
+The link:#schedule-configuration-interval[interval] for running
+the project_list cache warmer.
+
+By default, if `cache.project_list.maxAge` is set, `interval` will be set to
+half its value. If `cache.project_list.maxAge` is not set or `interval` is set
+to `-1`, it is disabled.
+
+[[cache.project_list.startTime]]cache.project_list.startTime::
++
+The link:#schedule-configuration-startTime[start time] for running
+the project_list cache warmer.
+
+Default is 00:00 if the project_list cache warmer is enabled.
[[capability]]
=== Section capability
@@ -3989,6 +4004,46 @@
+
Defaults to true.
+[[metrics]]
+=== Section metrics
+
+[[metrics.reservoir]]metrics.reservoir::
++
+The type of data reservoir used by the metrics system to calculate the percentile
+values for timers and histograms.
+It can be set to one of the following values:
++
+* ExponentiallyDecaying: An exponentially-decaying random reservoir based on
+ Cormode et al's forward-decaying priority reservoir sampling method to produce
+ a statistically representative sampling reservoir, exponentially biased towards
+ newer entries.
+* SlidingTimeWindowArray: A sliding window that stores only the measurements made
+ in the last window using chunks of 512 samples.
+* SlidingTimeWindow: A sliding window that stores only the measurements made in
+ the last window using a skip list.
+* SlidingWindow: A sliding window that stores only the last measurements.
+* Uniform: A random sampling reservoir that uses Vitter's Algorithm R to produce
+ a statistically representative sample.
++
+Defaults to ExponentiallyDecaying.
+
+[[metrics.ExponentiallyDecaying.alpha]]metrics.ExponentiallyDecaying.alpha::
++
+The exponential decay factor; the higher this is, the more biased the reservoir
+will be towards newer values.
+
+[[metrics.reservoirType.size]]metrics.<reservoirType>.size::
++
+The number of samples to keep in the reservoir. Applies to all reservoir types
+except the sliding time-based ones.
++
+Defaults to 1028.
+
+[[metrics.reservoirType.window]]metrics.<reservoirType>.window::
++
+The window of time for keeping data in the reservoir. It only applies to sliding
+time-based reservoir types.
+
[[mimetype]]
=== Section mimetype
diff --git a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
index 373246a..9a652e3 100644
--- a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
+++ b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
@@ -21,11 +21,13 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
import com.google.gerrit.server.config.AllProjectsConfigProvider;
import com.google.gerrit.server.config.FileBasedAllProjectsConfigProvider;
import com.google.gerrit.server.config.FileBasedGlobalPluginConfigProvider;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GlobalPluginConfigProvider;
+import com.google.gerrit.server.config.MetricsReservoirConfigImpl;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.config.TrackingFooters;
@@ -36,6 +38,7 @@
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
+import com.google.inject.Scopes;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -69,6 +72,7 @@
bind(InMemoryRepositoryManager.class).in(SINGLETON);
}
+ bind(MetricsReservoirConfig.class).to(MetricsReservoirConfigImpl.class).in(Scopes.SINGLETON);
bind(MetricMaker.class).to(TestMetricMaker.class);
listener().to(CreateSchema.class);
diff --git a/java/com/google/gerrit/index/query/AndPredicate.java b/java/com/google/gerrit/index/query/AndPredicate.java
index fd8ca96..23ae312 100644
--- a/java/com/google/gerrit/index/query/AndPredicate.java
+++ b/java/com/google/gerrit/index/query/AndPredicate.java
@@ -15,15 +15,19 @@
package com.google.gerrit.index.query;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
/** Requires all predicates to be true. */
-public class AndPredicate<T> extends Predicate<T> implements Matchable<T> {
+public class AndPredicate<T> extends Predicate<T>
+ implements Matchable<T>, Comparator<Predicate<T>> {
private final List<Predicate<T>> children;
private final int cost;
@@ -35,7 +39,7 @@
protected AndPredicate(Collection<? extends Predicate<T>> that) {
List<Predicate<T>> t = new ArrayList<>(that.size());
int c = 0;
- for (Predicate<T> p : that) {
+ for (Predicate<T> p : sort(that)) {
if (getClass() == p.getClass()) {
for (Predicate<T> gp : p.getChildren()) {
t.add(gp);
@@ -116,6 +120,28 @@
&& getChildren().equals(((Predicate<?>) other).getChildren());
}
+ private ImmutableList<Predicate<T>> sort(Collection<? extends Predicate<T>> that) {
+ return that.stream().sorted(this).collect(toImmutableList());
+ }
+
+ @Override
+ public int compare(Predicate<T> a, Predicate<T> b) {
+ int ai = a instanceof DataSource ? 0 : 1;
+ int bi = b instanceof DataSource ? 0 : 1;
+ int cmp = ai - bi;
+
+ if (cmp == 0) {
+ cmp = a.estimateCost() - b.estimateCost();
+ }
+
+ if (cmp == 0 && a instanceof DataSource && b instanceof DataSource) {
+ DataSource<?> as = (DataSource<?>) a;
+ DataSource<?> bs = (DataSource<?>) b;
+ cmp = as.getCardinality() - bs.getCardinality();
+ }
+ return cmp;
+ }
+
@Override
public String toString() {
final StringBuilder r = new StringBuilder();
diff --git a/java/com/google/gerrit/index/query/AndSource.java b/java/com/google/gerrit/index/query/AndSource.java
index de24df0..2a04051 100644
--- a/java/com/google/gerrit/index/query/AndSource.java
+++ b/java/com/google/gerrit/index/query/AndSource.java
@@ -15,23 +15,13 @@
package com.google.gerrit.index.query;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Ordering;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.IndexConfig;
-import com.google.gerrit.index.PaginationType;
-import com.google.gerrit.index.QueryOptions;
-import java.util.ArrayList;
import java.util.Collection;
-import java.util.Comparator;
import java.util.List;
-public class AndSource<T> extends AndPredicate<T>
- implements DataSource<T>, Comparator<Predicate<T>> {
+public class AndSource<T> extends AndPredicate<T> implements DataSource<T> {
protected final DataSource<T> source;
private final IsVisibleToPredicate<T> isVisibleToPredicate;
@@ -65,91 +55,34 @@
checkArgument(start >= 0, "negative start: %s", start);
this.isVisibleToPredicate = isVisibleToPredicate;
this.start = start;
+ this.indexConfig = indexConfig;
int c = Integer.MAX_VALUE;
DataSource<T> s = null;
int minCost = Integer.MAX_VALUE;
- for (Predicate<T> p : sort(getChildren())) {
+ for (Predicate<T> p : getChildren()) {
if (p instanceof DataSource) {
c = Math.min(c, ((DataSource<?>) p).getCardinality());
int cost = p.estimateCost();
if (cost < minCost) {
- s = toDataSource(p);
+ s = toPaginatingSource(p);
minCost = cost;
}
}
}
this.source = s;
this.cardinality = c;
- this.indexConfig = indexConfig;
}
@Override
public ResultSet<T> read() {
- if (source == null) {
- throw new StorageException("No DataSource: " + this);
- }
-
- // ResultSets are lazy. Calling #read here first and then dealing with ResultSets only when
- // requested allows the index to run asynchronous queries.
- ResultSet<T> resultSet = source.read();
- return new LazyResultSet<>(
- () -> {
- List<T> r = new ArrayList<>();
- T last = null;
- int pageResultSize = 0;
- for (T data : buffer(resultSet)) {
- if (!isMatchable() || match(data)) {
- r.add(data);
- }
- last = data;
- pageResultSize++;
- }
-
- if (last != null && source instanceof Paginated) {
- // Restart source and continue if we have not filled the
- // full limit the caller wants.
- //
- @SuppressWarnings("unchecked")
- Paginated<T> p = (Paginated<T>) source;
- QueryOptions opts = p.getOptions();
- final int limit = opts.limit();
- int pageSize = opts.pageSize();
- int pageSizeMultiplier = opts.pageSizeMultiplier();
- Object searchAfter = resultSet.searchAfter();
- int nextStart = pageResultSize;
- while (pageResultSize == pageSize && r.size() < limit) {
- pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
- ResultSet<T> next =
- indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
- ? p.restart(searchAfter, pageSize)
- : p.restart(nextStart, pageSize);
- pageResultSize = 0;
- for (T data : buffer(next)) {
- if (match(data)) {
- r.add(data);
- }
- pageResultSize++;
- }
- nextStart += pageResultSize;
- searchAfter = next.searchAfter();
- }
- }
-
- if (start >= r.size()) {
- return ImmutableList.of();
- } else if (start > 0) {
- return ImmutableList.copyOf(r.subList(start, r.size()));
- }
- return ImmutableList.copyOf(r);
- });
+ return source.read();
}
@Override
public ResultSet<FieldBundle> readRaw() {
- // TOOD(hiesel): Implement
- throw new UnsupportedOperationException("not implemented");
+ return source.readRaw();
}
@Override
@@ -170,11 +103,6 @@
return true;
}
- private Iterable<T> buffer(ResultSet<T> scanner) {
- return FluentIterable.from(Iterables.partition(scanner, 50))
- .transformAndConcat(this::transformBuffer);
- }
-
protected List<T> transformBuffer(List<T> buffer) {
return buffer;
}
@@ -184,46 +112,18 @@
return cardinality;
}
- private ImmutableList<Predicate<T>> sort(Collection<? extends Predicate<T>> that) {
- return that.stream().sorted(this).collect(toImmutableList());
- }
-
- @Override
- public int compare(Predicate<T> a, Predicate<T> b) {
- int ai = a instanceof DataSource ? 0 : 1;
- int bi = b instanceof DataSource ? 0 : 1;
- int cmp = ai - bi;
-
- if (cmp == 0) {
- cmp = a.estimateCost() - b.estimateCost();
- }
-
- if (cmp == 0 && a instanceof DataSource && b instanceof DataSource) {
- DataSource<?> as = (DataSource<?>) a;
- DataSource<?> bs = (DataSource<?>) b;
- cmp = as.getCardinality() - bs.getCardinality();
- }
- return cmp;
- }
-
@SuppressWarnings("unchecked")
- private DataSource<T> toDataSource(Predicate<T> pred) {
- return (DataSource<T>) pred;
- }
+ private PaginatingSource<T> toPaginatingSource(Predicate<T> pred) {
+ return new PaginatingSource<T>((DataSource<T>) pred, start, indexConfig) {
+ @Override
+ protected boolean match(T object) {
+ return AndSource.this.match(object);
+ }
- private int getNextPageSize(int pageSize, int pageSizeMultiplier) {
- List<Integer> possiblePageSizes = new ArrayList<>(3);
- try {
- possiblePageSizes.add(Math.multiplyExact(pageSize, pageSizeMultiplier));
- } catch (ArithmeticException e) {
- possiblePageSizes.add(Integer.MAX_VALUE);
- }
- if (indexConfig.maxPageSize() > 0) {
- possiblePageSizes.add(indexConfig.maxPageSize());
- }
- if (indexConfig.maxLimit() > 0) {
- possiblePageSizes.add(indexConfig.maxLimit());
- }
- return Ordering.natural().min(possiblePageSizes);
+ @Override
+ protected boolean isMatchable() {
+ return AndSource.this.isMatchable();
+ }
+ };
}
}
diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java
new file mode 100644
index 0000000..fd3a218
--- /dev/null
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -0,0 +1,148 @@
+// Copyright (C) 2022 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.index.query;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Ordering;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.index.PaginationType;
+import com.google.gerrit.index.QueryOptions;
+import java.util.ArrayList;
+import java.util.List;
+
+public class PaginatingSource<T> implements DataSource<T> {
+ protected final DataSource<T> source;
+ private final int start;
+ private final int cardinality;
+ private final IndexConfig indexConfig;
+
+ public PaginatingSource(DataSource<T> source, int start, IndexConfig indexConfig) {
+ checkArgument(start >= 0, "negative start: %s", start);
+ this.source = source;
+ this.start = start;
+ this.cardinality = source.getCardinality();
+ this.indexConfig = indexConfig;
+ }
+
+ @Override
+ public ResultSet<T> read() {
+ if (source == null) {
+ throw new StorageException("No DataSource: " + this);
+ }
+
+ // ResultSets are lazy. Calling #read here first and then dealing with ResultSets only when
+ // requested allows the index to run asynchronous queries.
+ ResultSet<T> resultSet = source.read();
+ return new LazyResultSet<>(
+ () -> {
+ List<T> r = new ArrayList<>();
+ T last = null;
+ int pageResultSize = 0;
+ for (T data : buffer(resultSet)) {
+ if (!isMatchable() || match(data)) {
+ r.add(data);
+ }
+ last = data;
+ pageResultSize++;
+ }
+
+ if (last != null && source instanceof Paginated) {
+ // Restart source and continue if we have not filled the
+ // full limit the caller wants.
+ //
+ @SuppressWarnings("unchecked")
+ Paginated<T> p = (Paginated<T>) source;
+ QueryOptions opts = p.getOptions();
+ final int limit = opts.limit();
+ int pageSize = opts.pageSize();
+ int pageSizeMultiplier = opts.pageSizeMultiplier();
+ Object searchAfter = resultSet.searchAfter();
+ int nextStart = pageResultSize;
+ while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit
+ pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
+ ResultSet<T> next =
+ indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
+ ? p.restart(searchAfter, pageSize)
+ : p.restart(nextStart, pageSize);
+ pageResultSize = 0;
+ for (T data : buffer(next)) {
+ if (match(data)) {
+ r.add(data);
+ }
+ pageResultSize++;
+ }
+ nextStart += pageResultSize;
+ searchAfter = next.searchAfter();
+ }
+ }
+
+ if (start >= r.size()) {
+ return ImmutableList.of();
+ } else if (start > 0) {
+ return ImmutableList.copyOf(r.subList(start, r.size()));
+ }
+ return ImmutableList.copyOf(r);
+ });
+ }
+
+ @Override
+ public ResultSet<FieldBundle> readRaw() {
+ // TOOD(hiesel): Implement
+ throw new UnsupportedOperationException("not implemented");
+ }
+
+ private Iterable<T> buffer(ResultSet<T> scanner) {
+ return FluentIterable.from(Iterables.partition(scanner, 50))
+ .transformAndConcat(this::transformBuffer);
+ }
+
+ protected boolean match(T object) {
+ return true;
+ }
+
+ protected boolean isMatchable() {
+ return true;
+ }
+
+ protected List<T> transformBuffer(List<T> buffer) {
+ return buffer;
+ }
+
+ @Override
+ public int getCardinality() {
+ return cardinality;
+ }
+
+ private int getNextPageSize(int pageSize, int pageSizeMultiplier) {
+ List<Integer> possiblePageSizes = new ArrayList<>(3);
+ try {
+ possiblePageSizes.add(Math.multiplyExact(pageSize, pageSizeMultiplier));
+ } catch (ArithmeticException e) {
+ possiblePageSizes.add(Integer.MAX_VALUE);
+ }
+ if (indexConfig.maxPageSize() > 0) {
+ possiblePageSizes.add(indexConfig.maxPageSize());
+ }
+ if (indexConfig.maxLimit() > 0) {
+ possiblePageSizes.add(indexConfig.maxLimit());
+ }
+ return Ordering.natural().min(possiblePageSizes);
+ }
+}
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index 72e8643..21d4c2e 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -79,7 +79,7 @@
private final IndexCollection<?, T, ? extends Index<?, T>> indexes;
private final IndexRewriter<T> rewriter;
private final String limitField;
- private final IntSupplier permittedLimit;
+ private final IntSupplier userQueryLimit;
private final CallerFinder callerFinder;
// This class is not generally thread-safe, but programmer error may result in it being shared
@@ -100,14 +100,14 @@
IndexCollection<?, T, ? extends Index<?, T>> indexes,
IndexRewriter<T> rewriter,
String limitField,
- IntSupplier permittedLimit) {
+ IntSupplier userQueryLimit) {
this.metrics = new Metrics(metricMaker);
this.schemaDef = schemaDef;
this.indexConfig = indexConfig;
this.indexes = indexes;
this.rewriter = rewriter;
this.limitField = limitField;
- this.permittedLimit = permittedLimit;
+ this.userQueryLimit = userQueryLimit;
this.used = new AtomicBoolean(false);
this.callerFinder =
CallerFinder.builder()
@@ -230,7 +230,7 @@
checkSupportedForQueries(q);
int limit = getEffectiveLimit(q);
limits.add(limit);
- int initialPageSize = getInitialPageSize(q);
+ int initialPageSize = getInitialPageSize(limit);
if (initialPageSize == getBackendSupportedLimit()) {
initialPageSize--;
@@ -281,6 +281,9 @@
@SuppressWarnings("unchecked")
DataSource<T> s = (DataSource<T>) pred;
+ if (initialPageSize < limit && !(pred instanceof AndSource)) {
+ s = new PaginatingSource<T>(s, start, indexConfig);
+ }
sources.add(s);
}
@@ -385,14 +388,17 @@
}
private int getPermittedLimit() {
- return enforceVisibility ? permittedLimit.getAsInt() : Integer.MAX_VALUE;
+ return enforceVisibility ? userQueryLimit.getAsInt() : Integer.MAX_VALUE;
}
private int getBackendSupportedLimit() {
return indexConfig.maxLimit();
}
- public int getInitialPageSize(Predicate<T> p) {
+ public int getEffectiveLimit(Predicate<T> p) {
+ if (isNoLimit == true) {
+ return Integer.MAX_VALUE;
+ }
List<Integer> possibleLimits = new ArrayList<>(4);
possibleLimits.add(getBackendSupportedLimit());
possibleLimits.add(getPermittedLimit());
@@ -407,18 +413,11 @@
}
int result = Ordering.natural().min(possibleLimits);
// Should have short-circuited from #query or thrown some other exception before getting here.
- checkState(result > 0, "effective initial page size should be positive");
+ checkState(result > 0, "effective limit should be positive");
return result;
}
- private int getEffectiveLimit(Predicate<T> p) {
- if (isNoLimit == true) {
- return Integer.MAX_VALUE;
- }
- return getInitialPageSize(p);
- }
-
private static Optional<QueryParseException> findQueryParseException(Throwable t) {
return Throwables.getCausalChain(t).stream()
.filter(c -> c instanceof QueryParseException)
@@ -426,6 +425,14 @@
.findFirst();
}
+ protected IntSupplier getUserQueryLimit() {
+ return userQueryLimit;
+ }
+
+ protected int getInitialPageSize(int queryLimit) {
+ return queryLimit;
+ }
+
protected abstract String formatForLogging(T t);
protected abstract int getIndexSize();
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 4d8c739..d40dfb4 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -40,6 +40,7 @@
import com.google.gerrit.entities.converter.ProtoConverter;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.FieldBundle;
@@ -322,6 +323,7 @@
private final QueryOptions opts;
private final Sort sort;
private final Function<Document, FieldBundle> rawDocumentMapper;
+ private final boolean isSearchAfterPagination;
private QuerySource(
List<ChangeSubIndex> indexes,
@@ -336,6 +338,8 @@
this.opts = opts;
this.sort = sort;
this.rawDocumentMapper = rawDocumentMapper;
+ this.isSearchAfterPagination =
+ opts.config().paginationType().equals(PaginationType.SEARCH_AFTER);
}
@Override
@@ -422,26 +426,34 @@
if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) {
realPageSize = Integer.MAX_VALUE;
}
- TopFieldDocs[] hits = new TopFieldDocs[indexes.size()];
+ List<TopFieldDocs> hits = new ArrayList<>();
+ int searchAfterHitsCount = 0;
for (int i = 0; i < indexes.size(); i++) {
ChangeSubIndex subIndex = indexes.get(i);
searchers[i] = subIndex.acquire();
- if (opts.searchAfter() != null && opts.searchAfter() instanceof HashMap) {
- hits[i] =
- searchers[i].searchAfter(
- ((HashMap<ChangeSubIndex, ScoreDoc>) opts.searchAfter()).get(subIndex),
- query,
- realPageSize,
- sort,
- /* doDocScores= */ false,
- /* doMaxScore= */ false);
+ if (isSearchAfterPagination) {
+ ScoreDoc searchAfter = getSearchAfter(subIndex);
+ int maxRemainingHits = realPageSize - searchAfterHitsCount;
+ if (maxRemainingHits > 0) {
+ TopFieldDocs subIndexHits =
+ searchers[i].searchAfter(
+ searchAfter,
+ query,
+ maxRemainingHits,
+ sort,
+ /* doDocScores= */ false,
+ /* doMaxScore= */ false);
+ searchAfterHitsCount += subIndexHits.scoreDocs.length;
+ hits.add(subIndexHits);
+ searchAfterBySubIndex.put(
+ subIndex, Iterables.getLast(Arrays.asList(subIndexHits.scoreDocs), searchAfter));
+ }
} else {
- hits[i] = searchers[i].search(query, realPageSize, sort);
+ hits.add(searchers[i].search(query, realPageSize, sort));
}
- searchAfterBySubIndex.put(
- subIndex, Iterables.getLast(Arrays.asList(hits[i].scoreDocs), null));
}
- TopDocs docs = TopDocs.merge(sort, realPageSize, hits);
+ TopDocs docs =
+ TopDocs.merge(sort, realPageSize, hits.stream().toArray(TopFieldDocs[]::new));
List<Document> result = new ArrayList<>(docs.scoreDocs.length);
for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
@@ -461,6 +473,23 @@
}
}
}
+
+ /**
+ * Returns null for the first page or when pagination type is not {@link
+ * PaginationType#SEARCH_AFTER search-after}, otherwise returns the last doc from previous
+ * search on the given change sub-index.
+ *
+ * @param subIndex change sub-index
+ * @return the score doc that can be used to page result sets
+ */
+ private ScoreDoc getSearchAfter(ChangeSubIndex subIndex) {
+ if (isSearchAfterPagination
+ && opts.searchAfter() != null
+ && opts.searchAfter() instanceof Map) {
+ return ((Map<ChangeSubIndex, ScoreDoc>) opts.searchAfter()).get(subIndex);
+ }
+ return null;
+ }
}
private static class Results {
diff --git a/java/com/google/gerrit/metrics/MetricsReservoirConfig.java b/java/com/google/gerrit/metrics/MetricsReservoirConfig.java
new file mode 100644
index 0000000..ca4cb09
--- /dev/null
+++ b/java/com/google/gerrit/metrics/MetricsReservoirConfig.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2022 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.metrics;
+
+import java.time.Duration;
+
+/** Configuration of the Metrics' reservoir type and size. */
+public interface MetricsReservoirConfig {
+
+ /** Returns the reservoir type. */
+ ReservoirType reservoirType();
+
+ /** Returns the reservoir window duration. */
+ Duration reservoirWindow();
+
+ /** Returns the number of samples that the reservoir can contain */
+ int reservoirSize();
+
+ /** Returns the alpha parameter of the ExponentiallyDecaying reservoir */
+ double reservoirAlpha();
+}
diff --git a/java/com/google/gerrit/metrics/ReservoirType.java b/java/com/google/gerrit/metrics/ReservoirType.java
new file mode 100644
index 0000000..fe89752
--- /dev/null
+++ b/java/com/google/gerrit/metrics/ReservoirType.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2022 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.metrics;
+
+/** Type of reservoir for collecting metrics into. */
+public enum ReservoirType {
+ ExponentiallyDecaying,
+ SlidingTimeWindowArray,
+ SlidingTimeWindow,
+ SlidingWindow,
+ Uniform;
+}
diff --git a/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java b/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
index fcba0ee..32be18d 100644
--- a/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
+++ b/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
@@ -18,8 +18,10 @@
import static com.google.gerrit.metrics.dropwizard.MetricResource.METRIC_KIND;
import static com.google.gerrit.server.config.ConfigResource.CONFIG_KIND;
+import com.codahale.metrics.Histogram;
import com.codahale.metrics.Metric;
import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -41,6 +43,7 @@
import com.google.gerrit.metrics.Histogram2;
import com.google.gerrit.metrics.Histogram3;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.metrics.Timer2;
@@ -48,10 +51,12 @@
import com.google.gerrit.metrics.proc.JGitMetricModule;
import com.google.gerrit.metrics.proc.ProcMetricModule;
import com.google.gerrit.server.cache.CacheMetrics;
+import com.google.gerrit.server.config.MetricsReservoirConfigImpl;
import com.google.inject.Inject;
import com.google.inject.Scopes;
import com.google.inject.Singleton;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
@@ -65,8 +70,25 @@
@Singleton
public class DropWizardMetricMaker extends MetricMaker {
public static class ApiModule extends RestApiModule {
+ private final Optional<MetricsReservoirConfig> metricsReservoirConfig;
+
+ public ApiModule(MetricsReservoirConfig metricsReservoirConfig) {
+ this.metricsReservoirConfig = Optional.of(metricsReservoirConfig);
+ }
+
+ public ApiModule() {
+ this.metricsReservoirConfig = Optional.empty();
+ }
+
@Override
protected void configure() {
+ if (metricsReservoirConfig.isPresent()) {
+ bind(MetricsReservoirConfig.class).toInstance(metricsReservoirConfig.get());
+ } else {
+ bind(MetricsReservoirConfig.class)
+ .to(MetricsReservoirConfigImpl.class)
+ .in(Scopes.SINGLETON);
+ }
bind(MetricRegistry.class).in(Scopes.SINGLETON);
bind(DropWizardMetricMaker.class).in(Scopes.SINGLETON);
bind(MetricMaker.class).to(DropWizardMetricMaker.class);
@@ -89,12 +111,14 @@
private final MetricRegistry registry;
private final Map<String, BucketedMetric> bucketed;
private final Map<String, ImmutableMap<String, String>> descriptions;
+ private final MetricsReservoirConfig reservoirConfig;
@Inject
- DropWizardMetricMaker(MetricRegistry registry) {
+ DropWizardMetricMaker(MetricRegistry registry, MetricsReservoirConfig reservoirConfig) {
this.registry = registry;
this.bucketed = new ConcurrentHashMap<>();
this.descriptions = new ConcurrentHashMap<>();
+ this.reservoirConfig = reservoirConfig;
}
Iterable<String> getMetricNames() {
@@ -222,7 +246,9 @@
}
TimerImpl newTimerImpl(String name) {
- return new TimerImpl(name, registry.timer(name));
+ return new TimerImpl(
+ name,
+ registry.timer(name, () -> new Timer(DropWizardReservoirProvider.get(reservoirConfig))));
}
@Override
@@ -271,7 +297,10 @@
}
HistogramImpl newHistogramImpl(String name) {
- return new HistogramImpl(name, registry.histogram(name));
+ return new HistogramImpl(
+ name,
+ registry.histogram(
+ name, () -> new Histogram(DropWizardReservoirProvider.get(reservoirConfig))));
}
@Override
diff --git a/java/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProvider.java b/java/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProvider.java
new file mode 100644
index 0000000..3089068
--- /dev/null
+++ b/java/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProvider.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2022 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.metrics.dropwizard;
+
+import com.codahale.metrics.ExponentiallyDecayingReservoir;
+import com.codahale.metrics.Reservoir;
+import com.codahale.metrics.SlidingTimeWindowArrayReservoir;
+import com.codahale.metrics.SlidingTimeWindowReservoir;
+import com.codahale.metrics.SlidingWindowReservoir;
+import com.codahale.metrics.UniformReservoir;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import java.util.concurrent.TimeUnit;
+
+class DropWizardReservoirProvider {
+
+ private DropWizardReservoirProvider() {}
+
+ static Reservoir get(MetricsReservoirConfig config) {
+ ReservoirType reservoirType = config.reservoirType();
+ switch (reservoirType) {
+ case ExponentiallyDecaying:
+ return new ExponentiallyDecayingReservoir(config.reservoirSize(), config.reservoirAlpha());
+ case SlidingTimeWindowArray:
+ return new SlidingTimeWindowArrayReservoir(
+ config.reservoirWindow().toMillis(), TimeUnit.MILLISECONDS);
+ case SlidingTimeWindow:
+ return new SlidingTimeWindowReservoir(
+ config.reservoirWindow().toMillis(), TimeUnit.MILLISECONDS);
+ case SlidingWindow:
+ return new SlidingWindowReservoir(config.reservoirSize());
+ case Uniform:
+ return new UniformReservoir(config.reservoirSize());
+
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported metrics reservoir type " + reservoirType.name());
+ }
+ }
+}
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 41ed991..9c0a4be 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -56,6 +56,7 @@
import com.google.gerrit.server.config.DefaultUrlFormatter.DefaultUrlFormatterModule;
import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
import com.google.gerrit.server.config.EnablePeerIPInReflogRecordProvider;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.config.SysExecutorModule;
@@ -95,6 +96,7 @@
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.inject.Injector;
+import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.TypeLiteral;
import com.google.inject.util.Providers;
@@ -102,6 +104,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Set;
+import org.eclipse.jgit.lib.Config;
/** Module for programs that perform batch operations on a site. */
public class BatchProgramModule extends FactoryModule {
@@ -204,7 +207,7 @@
// Submit rules
DynamicSet.setOf(binder(), SubmitRule.class);
factory(SubmitRuleEvaluator.Factory.class);
- modules.add(new PrologModule());
+ modules.add(new PrologModule(getConfig()));
modules.add(new DefaultSubmitRuleModule());
modules.add(new IgnoreSelfApprovalRuleModule());
@@ -226,4 +229,8 @@
.stream()
.forEach(this::install);
}
+
+ protected Config getConfig() {
+ return parentInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
+ }
}
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index f672d11..3b104dd 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -93,7 +93,7 @@
Duration refreshAfterWrite = def.refreshAfterWrite();
if (has(def.configKey(), "refreshAfterWrite")) {
- builder.expireAfterAccess(
+ builder.refreshAfterWrite(
ConfigUtil.getTimeUnit(
cfg,
"cache",
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 86d81e7..493b5b4 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -280,7 +280,7 @@
install(new GroupDbModule());
install(new GroupModule());
install(new NoteDbModule());
- install(new PrologModule());
+ install(new PrologModule(cfg));
install(new DefaultSubmitRuleModule());
install(new IgnoreSelfApprovalRuleModule());
install(new ReceiveCommitsModule());
diff --git a/java/com/google/gerrit/server/config/MetricsReservoirConfigImpl.java b/java/com/google/gerrit/server/config/MetricsReservoirConfigImpl.java
new file mode 100644
index 0000000..ac3c53a
--- /dev/null
+++ b/java/com/google/gerrit/server/config/MetricsReservoirConfigImpl.java
@@ -0,0 +1,96 @@
+// Copyright (C) 2022 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.config;
+
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.time.Duration;
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Config;
+
+/** Define metrics reservoir settings based on gerrit.config */
+@Singleton
+public class MetricsReservoirConfigImpl implements MetricsReservoirConfig {
+ private static final double RESERVOIR_ALPHA_DEFAULT = 0.015;
+ private static final int METRICS_RESERVOIR_SIZE_DEFAULT = 1028;
+ private static final long METRICS_RESERVOIR_WINDOW_MSEC_DEFAULT = 60000L;
+ private static final String METRICS_SECTION = "metrics";
+ private static final String METRICS_RESERVOIR = "reservoir";
+
+ private final ReservoirType reservoirType;
+
+ private final Duration reservoirWindow;
+ private final int reservoirSize;
+ private final double reservoirAlpha;
+
+ @Inject
+ MetricsReservoirConfigImpl(@GerritServerConfig Config gerritConfig) {
+ this.reservoirType =
+ gerritConfig.getEnum(
+ METRICS_SECTION, null, METRICS_RESERVOIR, ReservoirType.ExponentiallyDecaying);
+
+ reservoirWindow =
+ Duration.ofMillis(
+ ConfigUtil.getTimeUnit(
+ gerritConfig,
+ METRICS_SECTION,
+ reservoirType.name(),
+ "window",
+ METRICS_RESERVOIR_WINDOW_MSEC_DEFAULT,
+ TimeUnit.MILLISECONDS));
+ reservoirSize =
+ gerritConfig.getInt(
+ METRICS_SECTION, reservoirType.name(), "size", METRICS_RESERVOIR_SIZE_DEFAULT);
+ reservoirAlpha =
+ Optional.ofNullable(gerritConfig.getString(METRICS_SECTION, reservoirType.name(), "alpha"))
+ .map(Double::parseDouble)
+ .orElse(RESERVOIR_ALPHA_DEFAULT);
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirType()
+ */
+ @Override
+ public ReservoirType reservoirType() {
+ return reservoirType;
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirWindow()
+ */
+ @Override
+ public Duration reservoirWindow() {
+ return reservoirWindow;
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirSize()
+ */
+ @Override
+ public int reservoirSize() {
+ return reservoirSize;
+ }
+
+ /* (non-Javadoc)
+ * @see com.google.gerrit.server.config.MetricsConfig#reservoirAlpha()
+ */
+ @Override
+ public double reservoirAlpha() {
+ return reservoirAlpha;
+ }
+}
diff --git a/java/com/google/gerrit/server/events/EventTypes.java b/java/com/google/gerrit/server/events/EventTypes.java
index 5498ec8..229ef86 100644
--- a/java/com/google/gerrit/server/events/EventTypes.java
+++ b/java/com/google/gerrit/server/events/EventTypes.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.events;
+import com.google.common.collect.ImmutableMap;
import java.util.HashMap;
import java.util.Map;
@@ -61,4 +62,15 @@
public static Class<?> getClass(String type) {
return typesByString.get(type);
}
+
+ /**
+ * Get a copy of all currently registered events.
+ *
+ * <p>The key is the one given to the evenType parameter of the {@link #register} method.
+ *
+ * @return ImmutableMap of event types, Event classes.
+ */
+ public static Map<String, Class<?>> getRegisteredEvents() {
+ return ImmutableMap.copyOf(typesByString);
+ }
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index c06bbbf..f212384 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1457,7 +1457,7 @@
// Must pass explicit user instead of injecting a provider into CreateRefControl, since
// Provider<CurrentUser> within ReceiveCommits will always return anonymous.
createRefControl.checkCreateRef(
- Providers.of(user), receivePack.getRepository(), branch, obj);
+ Providers.of(user), receivePack.getRepository(), branch, obj, /* forPush= */ true);
} catch (AuthException denied) {
rejectProhibited(cmd, denied);
return;
diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java
index 0d015d4..ab134b5 100644
--- a/java/com/google/gerrit/server/project/CreateRefControl.java
+++ b/java/com/google/gerrit/server/project/CreateRefControl.java
@@ -25,10 +25,13 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.RetryHelper;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.List;
import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
@@ -41,18 +44,24 @@
/** Manages access control for creating Git references (aka branches, tags). */
@Singleton
public class CreateRefControl {
+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final Reachable reachable;
+ private final RetryHelper retryHelper;
@Inject
CreateRefControl(
- PermissionBackend permissionBackend, ProjectCache projectCache, Reachable reachable) {
+ PermissionBackend permissionBackend,
+ ProjectCache projectCache,
+ Reachable reachable,
+ RetryHelper retryHelper) {
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.reachable = reachable;
+ this.retryHelper = retryHelper;
}
/**
@@ -60,30 +69,56 @@
*
* @param user the user performing the operation
* @param repo repository on which user want to create
- * @param branch the branch the new {@link RevObject} should be created on
+ * @param destBranch the branch the new {@link RevObject} should be created on
* @param object the object the user will start the reference with
+ * @param sourceBranches the source ref from which the new ref is created from
* @throws AuthException if creation is denied; the message explains the denial.
* @throws PermissionBackendException on failure of permission checks.
* @throws ResourceConflictException if the project state does not permit the operation
*/
public void checkCreateRef(
- Provider<? extends CurrentUser> user, Repository repo, BranchNameKey branch, RevObject object)
+ Provider<? extends CurrentUser> user,
+ Repository repo,
+ BranchNameKey destBranch,
+ RevObject object,
+ boolean forPush,
+ BranchNameKey... sourceBranches)
throws AuthException, PermissionBackendException, NoSuchProjectException, IOException,
ResourceConflictException {
ProjectState ps =
- projectCache.get(branch.project()).orElseThrow(noSuchProject(branch.project()));
+ projectCache.get(destBranch.project()).orElseThrow(noSuchProject(destBranch.project()));
ps.checkStatePermitsWrite();
- PermissionBackend.ForRef perm = permissionBackend.user(user.get()).ref(branch);
+ PermissionBackend.ForRef perm = permissionBackend.user(user.get()).ref(destBranch);
if (object instanceof RevCommit) {
perm.check(RefPermission.CREATE);
- checkCreateCommit(user, repo, (RevCommit) object, ps.getNameKey(), perm);
+ if (sourceBranches.length == 0) {
+ checkCreateCommit(user, repo, (RevCommit) object, ps.getNameKey(), perm, forPush);
+ } else {
+ for (BranchNameKey src : sourceBranches) {
+ PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(src);
+ if (forRef.testOrFalse(RefPermission.READ)) {
+ return;
+ }
+ }
+ AuthException e =
+ new AuthException(
+ String.format(
+ "must have %s on existing ref to create new ref from it",
+ RefPermission.READ.describeForException()));
+ e.setAdvice(
+ String.format(
+ "use an existing ref visible to you, or get %s permission on the ref",
+ RefPermission.READ.describeForException()));
+ throw e;
+ }
} else if (object instanceof RevTag) {
RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) {
rw.parseBody(tag);
} catch (IOException e) {
- logger.atSevere().withCause(e).log("RevWalk(%s) parsing %s:", branch.project(), tag.name());
+ logger.atSevere().withCause(e).log(
+ "RevWalk(%s) parsing %s:", destBranch.project(), tag.name());
throw e;
}
@@ -97,14 +132,14 @@
RevObject target = tag.getObject();
if (target instanceof RevCommit) {
- checkCreateCommit(user, repo, (RevCommit) target, ps.getNameKey(), perm);
+ checkCreateCommit(user, repo, (RevCommit) target, ps.getNameKey(), perm, forPush);
} else {
- checkCreateRef(user, repo, branch, target);
+ checkCreateRef(user, repo, destBranch, target, forPush);
}
// If the tag has a PGP signature, allow a lower level of permission
// than if it doesn't have a PGP signature.
- PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(branch);
+ PermissionBackend.ForRef forRef = permissionBackend.user(user.get()).ref(destBranch);
if (tag.getRawGpgSignature() != null) {
forRef.check(RefPermission.CREATE_SIGNED_TAG);
} else {
@@ -122,12 +157,33 @@
Repository repo,
RevCommit commit,
Project.NameKey project,
- PermissionBackend.ForRef forRef)
+ PermissionBackend.ForRef forRef,
+ boolean forPush)
throws AuthException, PermissionBackendException, IOException {
- // If the user has update (push) permission, they can create the ref regardless
- // of whether they are pushing any new objects along with the create.
- if (forRef.test(RefPermission.UPDATE)) {
- return;
+ try {
+ // If the user has UPDATE (push) permission, they can set the ref to an arbitrary commit:
+ //
+ // * if they don't have access, we don't advertise the data, and a conforming git client
+ // would send the object along with the push as outcome of the negotation.
+ // * a malicious client could try to send the update without sending the object. This
+ // is prevented by JGit's ConnectivityChecker (see receive.checkReferencedObjectsAreReachable
+ // to switch off this costly check).
+ //
+ // Thus, when using the git command-line client, we don't need to do extra checks for users
+ // with push access.
+ //
+ // When using the REST API, there is no negotiation, and the target commit must already be on
+ // the server, so we must check that the user can see that commit.
+ if (forPush) {
+ // We can only shortcut for UPDATE permission. Pushing a tag (CREATE_TAG, CREATE_SIGNED_TAG)
+ // can also introduce new objects. While there may not be a confidentiality problem
+ // (the caller supplies the data as documented above), the permission is for creating
+ // tags to existing commits.
+ forRef.check(RefPermission.UPDATE);
+ return;
+ }
+ } catch (AuthException denied) {
+ // Fall through to check reachability.
}
if (reachable.fromRefs(
project,
@@ -142,6 +198,18 @@
return;
}
+ // Previous check only catches normal branches. Try PatchSet refs too. If we can create refs,
+ // we're not a replica, so we can always use the change index.
+ List<ChangeData> changes =
+ retryHelper
+ .changeIndexQuery(
+ "queryChangesByProjectCommitWithLimit1",
+ q -> q.enforceVisibility(true).setLimit(1).byProjectCommit(project, commit))
+ .call();
+ if (!changes.isEmpty()) {
+ return;
+ }
+
AuthException e =
new AuthException(
String.format(
diff --git a/java/com/google/gerrit/server/project/PeriodicProjectListCacheWarmer.java b/java/com/google/gerrit/server/project/PeriodicProjectListCacheWarmer.java
new file mode 100644
index 0000000..caffb45
--- /dev/null
+++ b/java/com/google/gerrit/server/project/PeriodicProjectListCacheWarmer.java
@@ -0,0 +1,100 @@
+// Copyright (C) 2022 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.project;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.ScheduleConfig;
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.inject.Inject;
+import java.time.Duration;
+import org.eclipse.jgit.lib.Config;
+
+public class PeriodicProjectListCacheWarmer implements Runnable {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ public static class LifeCycle implements LifecycleListener {
+ protected final Config config;
+ protected final WorkQueue queue;
+ protected final PeriodicProjectListCacheWarmer runner;
+
+ @Inject
+ LifeCycle(
+ @GerritServerConfig Config config, WorkQueue queue, PeriodicProjectListCacheWarmer runner) {
+ this.config = config;
+ this.queue = queue;
+ this.runner = runner;
+ }
+
+ @Override
+ public void start() {
+ long interval = -1L;
+ String intervalString = config.getString("cache", ProjectCacheImpl.CACHE_LIST, "interval");
+ if (!"-1".equals(intervalString)) {
+ long maxAge =
+ config.getTimeUnit("cache", ProjectCacheImpl.CACHE_LIST, "maxAge", -1L, MILLISECONDS);
+ interval =
+ config.getTimeUnit(
+ "cache",
+ ProjectCacheImpl.CACHE_LIST,
+ "interval",
+ getHalfDuration(maxAge),
+ MILLISECONDS);
+ }
+
+ if (interval == -1L) {
+ logger.atWarning().log("project_list cache warmer is disabled");
+ return;
+ }
+
+ String startTime = config.getString("cache", ProjectCacheImpl.CACHE_LIST, "startTime");
+ if (startTime == null) {
+ startTime = "00:00";
+ }
+
+ runner.run();
+ queue.scheduleAtFixedRate(runner, ScheduleConfig.Schedule.createOrFail(interval, startTime));
+ }
+
+ @Override
+ public void stop() {
+ // handled by WorkQueue.stop() already
+ }
+
+ private long getHalfDuration(long duration) {
+ if (duration < 0) {
+ return duration;
+ }
+ return Duration.ofMillis(duration).dividedBy(2L).toMillis();
+ }
+ }
+
+ protected final ProjectCache cache;
+
+ @Inject
+ PeriodicProjectListCacheWarmer(ProjectCache cache) {
+ this.cache = cache;
+ }
+
+ @Override
+ public void run() {
+ logger.atFine().log("Loading project_list cache");
+ cache.all();
+ logger.atFine().log("Finished loading project_list cache");
+ }
+}
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 3aa3783..52a524f 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -95,7 +95,7 @@
public static final String PERSISTED_CACHE_NAME = "persisted_projects";
- private static final String CACHE_LIST = "project_list";
+ public static final String CACHE_LIST = "project_list";
public static Module module() {
return new CacheModule() {
@@ -147,6 +147,13 @@
listener().to(ProjectCacheWarmer.class);
}
});
+ install(
+ new LifecycleModule() {
+ @Override
+ protected void configure() {
+ listener().to(PeriodicProjectListCacheWarmer.LifeCycle.class);
+ }
+ });
}
};
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 7a2dc44a..0f0535a 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -160,4 +160,9 @@
protected int getBatchSize() {
return sequences.changeBatchSize();
}
+
+ @Override
+ protected int getInitialPageSize(int limit) {
+ return Math.min(getUserQueryLimit().getAsInt(), limit);
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
index c9da5b8..018ed86 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -50,6 +50,7 @@
import java.util.Map;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevObject;
@@ -132,7 +133,21 @@
object = rw.parseCommit(object);
}
- createRefControl.checkCreateRef(identifiedUser, repo, name, object);
+ Ref sourceRef = repo.exactRef(input.revision);
+ if (sourceRef == null) {
+ createRefControl.checkCreateRef(identifiedUser, repo, name, object, /* forPush= */ false);
+ } else {
+ if (sourceRef.isSymbolic()) {
+ sourceRef = sourceRef.getTarget();
+ }
+ createRefControl.checkCreateRef(
+ identifiedUser,
+ repo,
+ name,
+ object,
+ /* forPush= */ false,
+ BranchNameKey.create(rsrc.getNameKey(), sourceRef.getName()));
+ }
RefUpdate u = repo.updateRef(ref);
u.setExpectedOldObjectId(ObjectId.zeroId());
diff --git a/java/com/google/gerrit/server/rules/PrologModule.java b/java/com/google/gerrit/server/rules/PrologModule.java
index 5cf4220..ebb5ec0 100644
--- a/java/com/google/gerrit/server/rules/PrologModule.java
+++ b/java/com/google/gerrit/server/rules/PrologModule.java
@@ -18,12 +18,19 @@
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.rules.RulesCache.RulesCacheModule;
+import org.eclipse.jgit.lib.Config;
public class PrologModule extends FactoryModule {
+ protected final Config config;
+
+ public PrologModule(Config config) {
+ this.config = config;
+ }
+
@Override
protected void configure() {
install(new EnvironmentModule());
- install(new RulesCacheModule());
+ install(new RulesCacheModule(config));
bind(PrologEnvironment.Args.class);
factory(PrologRuleEvaluator.Factory.class);
diff --git a/java/com/google/gerrit/server/rules/RulesCache.java b/java/com/google/gerrit/server/rules/RulesCache.java
index 706804a..773c75e 100644
--- a/java/com/google/gerrit/server/rules/RulesCache.java
+++ b/java/com/google/gerrit/server/rules/RulesCache.java
@@ -17,6 +17,7 @@
import static com.googlecode.prolog_cafe.lang.PrologMachineCopy.save;
import com.google.common.base.Joiner;
+import com.google.common.base.Strings;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Project;
@@ -73,11 +74,25 @@
@Singleton
public class RulesCache {
public static class RulesCacheModule extends CacheModule {
+ protected final Config config;
+
+ public RulesCacheModule(Config config) {
+ this.config = config;
+ }
+
@Override
protected void configure() {
- cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class)
- // This cache is auxiliary to the project cache, so size it the same.
- .configKey(ProjectCacheImpl.CACHE_NAME);
+ if (has(ProjectCacheImpl.CACHE_NAME, "memoryLimit")) {
+ // As this cache is auxiliary to the project cache, so size it the same when available
+ cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class)
+ .maximumWeight(config.getLong("cache", ProjectCacheImpl.CACHE_NAME, "memoryLimit", 0));
+ } else {
+ cache(RulesCache.CACHE_NAME, ObjectId.class, PrologMachineCopy.class);
+ }
+ }
+
+ private boolean has(String name, String var) {
+ return !Strings.isNullOrEmpty(config.getString("cache", name, var));
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index e76e2f6..f94aa12 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -47,6 +47,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
import org.eclipse.jgit.transport.TrackingRefUpdate;
import org.junit.Before;
@@ -94,6 +95,32 @@
}
@Test
+ public void pushNewCommitsRequiresPushPermission() throws Exception {
+ testRepo.branch("HEAD").commit().create();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ PushResult r = push("HEAD:refs/heads/newbranch");
+
+ String msg = "update for creating new commit object not permitted";
+ RemoteRefUpdate rru = r.getRemoteUpdate("refs/heads/newbranch");
+ assertThat(rru.getStatus()).isNotEqualTo(Status.OK);
+ assertThat(rru.getMessage()).contains(msg);
+
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ RemoteRefUpdate success =
+ push("HEAD:refs/heads/newbranch").getRemoteUpdate("refs/heads/newbranch");
+ assertThat(success.getStatus()).isEqualTo(Status.OK);
+ }
+
+ @Test
public void fastForwardUpdateDenied() throws Exception {
testRepo.branch("HEAD").commit().create();
PushResult r = push("HEAD:refs/heads/master");
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index 7c51a42..df899ce 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -31,7 +31,10 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Account;
@@ -67,6 +70,7 @@
public class CreateBranchIT extends AbstractDaemonTest {
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private GroupOperations groupOperations;
@Inject private ExtensionRegistry extensionRegistry;
private BranchNameKey testBranch;
@@ -497,6 +501,50 @@
}
}
+ @Test
+ public void createBranchRevisionVisibility() throws Exception {
+ AccountGroup.UUID privilegedGroupUuid =
+ groupOperations.newGroup().name(name("privilegedGroup")).create();
+ TestAccount privilegedUser =
+ accountCreator.create(
+ "privilegedUser", "privilegedUser@example.com", "privilegedUser", null);
+ groupOperations.group(privilegedGroupUuid).forUpdate().addMember(privilegedUser.id()).update();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/heads/secret/*").group(REGISTERED_USERS))
+ .add(allow(Permission.READ).ref("refs/heads/secret/*").group(privilegedGroupUuid))
+ .add(allow(Permission.READ).ref("refs/heads/*").group(REGISTERED_USERS))
+ .add(allow(Permission.CREATE).ref("refs/heads/*").group(REGISTERED_USERS))
+ .add(allow(Permission.PUSH).ref("refs/heads/*").group(REGISTERED_USERS))
+ .update();
+ PushOneCommit push =
+ pushFactory.create(admin.newIdent(), testRepo, "Configure", "file.txt", "contents");
+ PushOneCommit.Result result = push.to("refs/heads/secret/main");
+ result.assertOkStatus();
+ RevCommit secretCommit = result.getCommit();
+ requestScopeOperations.setApiUser(privilegedUser.id());
+ BranchInfo info = gApi.projects().name(project.get()).branch("refs/heads/secret/main").get();
+ assertThat(info.revision).isEqualTo(secretCommit.name());
+ TestAccount unprivileged =
+ accountCreator.create("unprivileged", "unprivileged@example.com", "unprivileged", null);
+ requestScopeOperations.setApiUser(unprivileged.id());
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> gApi.projects().name(project.get()).branch("refs/heads/secret/main").get());
+ BranchInput branchInput = new BranchInput();
+ branchInput.ref = "public";
+ branchInput.revision = secretCommit.name();
+ assertThrows(
+ AuthException.class,
+ () -> gApi.projects().name(project.get()).branch(branchInput.ref).create(branchInput));
+
+ branchInput.revision = "refs/heads/secret/main";
+ assertThrows(
+ AuthException.class,
+ () -> gApi.projects().name(project.get()).branch(branchInput.ref).create(branchInput));
+ }
+
private void blockCreateReference() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/index/query/AndSourceTest.java b/javatests/com/google/gerrit/index/query/AndSourceTest.java
new file mode 100644
index 0000000..3ae48fb
--- /dev/null
+++ b/javatests/com/google/gerrit/index/query/AndSourceTest.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2022 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.index.query;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.Lists;
+import org.junit.Test;
+
+public class AndSourceTest extends PredicateTest {
+ @Test
+ public void ensureLowerCostPredicateRunsFirst() {
+ TestMatchablePredicate p1 = new TestMatchablePredicate("predicate1", "foo", 10);
+ TestMatchablePredicate p2 = new TestMatchablePredicate("predicate2", "foo", 1);
+ AndSource<String> andSource = new AndSource<>(Lists.newArrayList(p1, p2), null);
+ andSource.match("bar");
+ assertFalse(p1.ranMatch);
+ assertTrue(p2.ranMatch);
+ }
+}
diff --git a/javatests/com/google/gerrit/index/query/PredicateTest.java b/javatests/com/google/gerrit/index/query/PredicateTest.java
index 171ca27..8cb8d17 100644
--- a/javatests/com/google/gerrit/index/query/PredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/PredicateTest.java
@@ -18,7 +18,30 @@
@Ignore
public abstract class PredicateTest {
- protected static final class TestPredicate extends OperatorPredicate<String> {
+ @SuppressWarnings("ProtectedMembersInFinalClass")
+ protected static final class TestMatchablePredicate extends TestPredicate
+ implements Matchable<String> {
+ protected int cost;
+ protected boolean ranMatch = false;
+
+ protected TestMatchablePredicate(String name, String value, int cost) {
+ super(name, value);
+ this.cost = cost;
+ }
+
+ @Override
+ public boolean match(String object) {
+ ranMatch = true;
+ return false;
+ }
+
+ @Override
+ public int getCost() {
+ return cost;
+ }
+ }
+
+ protected static class TestPredicate extends OperatorPredicate<String> {
private TestPredicate(String name, String value) {
super(name, value);
}
diff --git a/javatests/com/google/gerrit/metrics/dropwizard/BUILD b/javatests/com/google/gerrit/metrics/dropwizard/BUILD
index 98d12b2..e236f30 100644
--- a/javatests/com/google/gerrit/metrics/dropwizard/BUILD
+++ b/javatests/com/google/gerrit/metrics/dropwizard/BUILD
@@ -6,7 +6,10 @@
tags = ["metrics"],
visibility = ["//visibility:public"],
deps = [
+ "//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/metrics/dropwizard",
+ "//lib/mockito",
"//lib/truth",
+ "@dropwizard-core//jar",
],
)
diff --git a/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java
index 9b21bf6..5777779 100644
--- a/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java
+++ b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardMetricMakerTest.java
@@ -15,12 +15,33 @@
package com.google.gerrit.metrics.dropwizard;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import com.codahale.metrics.MetricRegistry;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+@RunWith(MockitoJUnitRunner.class)
public class DropWizardMetricMakerTest {
- DropWizardMetricMaker metrics =
- new DropWizardMetricMaker(null /* MetricRegistry unused in tests */);
+
+ @Mock MetricsReservoirConfig reservoirConfigMock;
+
+ MetricRegistry registry;
+
+ DropWizardMetricMaker metrics;
+
+ @Before
+ public void setupMocks() {
+ registry = new MetricRegistry();
+ metrics = new DropWizardMetricMaker(registry, reservoirConfigMock);
+ }
@Test
public void shouldSanitizeUnwantedChars() throws Exception {
@@ -41,4 +62,15 @@
assertThat(metrics.sanitizeMetricName("metric//")).isEqualTo("metric");
assertThat(metrics.sanitizeMetricName("metric/submetric/")).isEqualTo("metric/submetric");
}
+
+ @Test
+ public void shouldRequestForReservoirForNewTimer() throws Exception {
+ when(reservoirConfigMock.reservoirType()).thenReturn(ReservoirType.ExponentiallyDecaying);
+
+ metrics.newTimer(
+ "foo",
+ new Description("foo description").setCumulative().setUnit(Description.Units.MILLISECONDS));
+
+ verify(reservoirConfigMock).reservoirType();
+ }
}
diff --git a/javatests/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProviderTest.java b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProviderTest.java
new file mode 100644
index 0000000..6402b53
--- /dev/null
+++ b/javatests/com/google/gerrit/metrics/dropwizard/DropWizardReservoirProviderTest.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2022 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.metrics.dropwizard;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
+
+import com.codahale.metrics.ExponentiallyDecayingReservoir;
+import com.codahale.metrics.SlidingTimeWindowArrayReservoir;
+import com.codahale.metrics.SlidingTimeWindowReservoir;
+import com.codahale.metrics.SlidingWindowReservoir;
+import com.codahale.metrics.UniformReservoir;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
+import com.google.gerrit.metrics.ReservoirType;
+import java.time.Duration;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DropWizardReservoirProviderTest {
+ private static final int SLIDING_WINDOW_INTERVAL = 1;
+ private static final int SLIDING_WINDOW_SIZE = 256;
+
+ @Mock private MetricsReservoirConfig configMock;
+
+ @Test
+ public void shouldInstantiateReservoirProviderBasedOnMetricsConfig() {
+ when(configMock.reservoirType()).thenReturn(ReservoirType.ExponentiallyDecaying);
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(ExponentiallyDecayingReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.SlidingTimeWindow);
+ when(configMock.reservoirWindow()).thenReturn(Duration.ofMinutes(1));
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(SlidingTimeWindowReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.SlidingTimeWindowArray);
+ when(configMock.reservoirWindow()).thenReturn(Duration.ofMinutes(SLIDING_WINDOW_INTERVAL));
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(SlidingTimeWindowArrayReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.SlidingWindow);
+ when(configMock.reservoirSize()).thenReturn(SLIDING_WINDOW_SIZE);
+ assertThat(DropWizardReservoirProvider.get(configMock))
+ .isInstanceOf(SlidingWindowReservoir.class);
+
+ when(configMock.reservoirType()).thenReturn(ReservoirType.Uniform);
+ assertThat(DropWizardReservoirProvider.get(configMock)).isInstanceOf(UniformReservoir.class);
+ }
+}
diff --git a/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java b/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
index 33919e7..ea89ae9 100644
--- a/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
+++ b/javatests/com/google/gerrit/metrics/proc/ProcMetricModuleTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.Mockito.mock;
import com.codahale.metrics.Counter;
import com.codahale.metrics.Gauge;
@@ -31,7 +32,9 @@
import com.google.gerrit.metrics.Description.FieldOrdering;
import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.MetricsReservoirConfig;
import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker;
+import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
@@ -179,7 +182,14 @@
@Before
public void setup() {
- Injector injector = Guice.createInjector(new DropWizardMetricMaker.ApiModule());
+ Injector injector =
+ Guice.createInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ install(new DropWizardMetricMaker.ApiModule(mock(MetricsReservoirConfig.class)));
+ }
+ });
LifecycleManager mgr = new LifecycleManager();
mgr.add(injector);
diff --git a/javatests/com/google/gerrit/server/events/EventTypesTest.java b/javatests/com/google/gerrit/server/events/EventTypesTest.java
index c822d6c..7e97f184 100644
--- a/javatests/com/google/gerrit/server/events/EventTypesTest.java
+++ b/javatests/com/google/gerrit/server/events/EventTypesTest.java
@@ -48,4 +48,16 @@
Class<?> clazz = EventTypes.getClass("does-not-exist-event");
assertThat(clazz).isNull();
}
+
+ @Test
+ public void getRegisteredEventsGetsANewlyRegisteredEvent() {
+ EventTypes.register(TestEvent.TYPE, TestEvent.class);
+ assertThat(EventTypes.getRegisteredEvents()).containsEntry(TestEvent.TYPE, TestEvent.class);
+ }
+
+ @Test
+ public void getRegisteredEventsGetsTypeGivenAtRegistration() {
+ EventTypes.register("alternate-type", TestEvent.class);
+ assertThat(EventTypes.getRegisteredEvents()).containsEntry("alternate-type", TestEvent.class);
+ }
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index 27e5b4f..3d0fd25 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
@@ -97,7 +97,7 @@
Predicate<ChangeData> in = parse("foo:a file:b");
Predicate<ChangeData> out = rewrite(in);
assertThat(AndChangeSource.class).isSameInstanceAs(out.getClass());
- assertThat(out.getChildren()).containsExactly(query(in.getChild(1)), in.getChild(0)).inOrder();
+ assertThat(out.getChildren()).containsExactly(query(parse("file:b")), parse("foo:a")).inOrder();
}
@Test
@@ -126,9 +126,9 @@
.inOrder();
// Same at the assertions above, that were added for readability
- assertThat(out.getChild(0)).isEqualTo(query(in.getChild(0)));
+ assertThat(out.getChild(0)).isEqualTo(query(parse("-status:abandoned")));
assertThat(indexedSubTree.getChildren())
- .containsExactly(query(in.getChild(1).getChild(1)), in.getChild(1).getChild(0))
+ .containsExactly(query(parse("file:b")), parse("foo:a"))
.inOrder();
}
@@ -137,7 +137,9 @@
Predicate<ChangeData> in = parse("-foo:a (file:b OR file:c)");
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isSameInstanceAs(AndChangeSource.class);
- assertThat(out.getChildren()).containsExactly(query(in.getChild(1)), in.getChild(0)).inOrder();
+ assertThat(out.getChildren())
+ .containsExactly(query(parse("file:b OR file:c")), parse("-foo:a"))
+ .inOrder();
}
@Test
@@ -146,7 +148,8 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isSameInstanceAs(OrSource.class);
assertThat(out.getChildren())
- .containsExactly(query(or(in.getChild(0), in.getChild(2))), in.getChild(1), in.getChild(3))
+ .containsExactly(
+ query(or(parse("file:a"), parse("file:c"))), parse("foo:b"), parse("foo:d"))
.inOrder();
}
@@ -156,7 +159,7 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(AndChangeSource.class).isSameInstanceAs(out.getClass());
assertThat(out.getChildren())
- .containsExactly(query(and(in.getChild(0), in.getChild(2))), in.getChild(1))
+ .containsExactly(query(and(parse("status:new"), parse("file:a"))), parse("bar:p"))
.inOrder();
}
@@ -166,7 +169,7 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isEqualTo(AndChangeSource.class);
assertThat(out.getChildren())
- .containsExactly(query(and(in.getChild(0), in.getChild(2))), in.getChild(1))
+ .containsExactly(query(and(parse("status:new"), parse("file:a"))), parse("bar:p"))
.inOrder();
}
@@ -176,7 +179,7 @@
Predicate<ChangeData> out = rewrite(in);
assertThat(out.getClass()).isEqualTo(AndChangeSource.class);
assertThat(out.getChildren())
- .containsExactly(query(and(in.getChild(0), in.getChild(2))), in.getChild(1))
+ .containsExactly(query(and(parse("status:new OR file:a"), parse("file:b"))), parse("bar:p"))
.inOrder();
}
@@ -186,7 +189,7 @@
Predicate<ChangeData> out = rewrite(in, options(0, 5));
assertThat(out.getClass()).isEqualTo(AndChangeSource.class);
assertThat(out.getChildren())
- .containsExactly(query(in.getChild(1), 5), parse("limit:5"), parse("limit:5"))
+ .containsExactly(query(parse("file:a"), 5), parse("limit:5"), parse("limit:5"))
.inOrder();
}
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 16f7199..e0a69a0 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -646,7 +646,10 @@
.getRaw(
Account.id(userInfo._accountId),
QueryOptions.create(
- IndexConfig.createDefault(), 0, 1, schema.getStoredFields().keySet()));
+ IndexConfig.fromConfig(config).build(),
+ 0,
+ 1,
+ schema.getStoredFields().keySet()));
assertThat(rawFields).isPresent();
if (schema.useLegacyNumericFields()) {
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index d3c6d84..32a646e 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -21,6 +21,7 @@
"//java/com/google/gerrit/acceptance/config",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/common:annotations",
+ "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/httpd",
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index 01ae960..3968a33 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -99,10 +99,37 @@
AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
- // 2 index searches are expected. The first index search will run with size 2 (i.e.
- // the configured query-limit), and then we will paginate to get the remaining 2
+ // 2 index searches are expected. The first index search will run with size 3 (i.e.
+ // the configured query-limit+1), and then we will paginate to get the remaining
// changes with the second index search.
newQuery("status:new").withNoLimit().get();
assertThat(idx.getQueryCount()).isEqualTo(2);
}
+
+ @Test
+ @UseClockStep
+ @SuppressWarnings("unchecked")
+ public void internalQueriesPaginate() throws Exception {
+ // create 4 changes
+ TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+
+ // Set queryLimit to 2
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
+ .update();
+
+ AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+
+ // 2 index searches are expected. The first index search will run with size 3 (i.e.
+ // the configured query-limit+1), and then we will paginate to get the remaining
+ // changes with the second index search.
+ queryProvider.get().query(queryBuilderProvider.get().parse("status:new"));
+ assertThat(idx.getQueryCount()).isEqualTo(2);
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
index 6a83fb9..9717bfb 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
@@ -15,13 +15,18 @@
package com.google.gerrit.server.query.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
+import static com.google.gerrit.common.data.GlobalCapability.QUERY_LIMIT;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.testing.InMemoryModule;
import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
import com.google.inject.Guice;
+import com.google.inject.Inject;
import com.google.inject.Injector;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -29,6 +34,8 @@
import org.junit.Test;
public abstract class LuceneQueryChangesTest extends AbstractQueryChangesTest {
+ @Inject protected AllProjectsName allProjects;
+
@Override
protected Injector createInjector() {
Config luceneConfig = new Config(config);
@@ -66,4 +73,29 @@
() -> assertQuery("owner: \"" + nameEmail + "\"\\", change1));
assertThat(thrown).hasMessageThat().contains("Cannot create full-text query with value: \\");
}
+
+ @Test
+ public void openAndClosedChanges() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+
+ // create 3 closed changes
+ Change change1 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
+ Change change2 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
+ Change change3 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
+
+ // create 3 new changes
+ Change change4 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+ Change change5 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+ Change change6 = insert(repo, newChangeWithStatus(repo, Change.Status.NEW));
+
+ // Set queryLimit to 1
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 1))
+ .update();
+
+ Change[] expected = new Change[] {change6, change5, change4, change3, change2, change1};
+ assertQuery(newQuery("project:repo").withNoLimit(), expected);
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index 568b5a0..6e4fec1 100644
--- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -374,7 +374,7 @@
.getRaw(
uuid,
QueryOptions.create(
- IndexConfig.createDefault(),
+ IndexConfig.fromConfig(config).build(),
0,
10,
indexes.getSearchIndex().getSchema().getStoredFields().keySet()));
diff --git a/modules/jgit b/modules/jgit
index 1bd087a..d013761 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 1bd087a7f515bcd944f8d8a1fbb26a0184bdccf6
+Subproject commit d01376106af8800017ac3c08d7c7ac1fd5ccc9ee
diff --git a/plugins/gitiles b/plugins/gitiles
index 44b5333..24529d2 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 44b5333d08d02c15334f81906134a5613d3f0a3b
+Subproject commit 24529d232268ac51fd6850770f70dc0fcd732dd8
