summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKaushik Lingarkar <kaushik.lingarkar@linaro.org>2022-09-13 10:16:15 -0700
committerKaushik Lingarkar <kaushik.lingarkar@linaro.org>2022-09-13 10:31:40 -0700
commit75096adc91e4344d58df117c32b9b75cbb649021 (patch)
tree02fdf2c165767934a194cb1759fe9861e23c38cd
parent7ee6cd3d289a224bfbf150a70d63ccf2112bc531 (diff)
parentb4b73bda1ff749c5f713f1444f563555de76048e (diff)
Merge branch 'stable-3.4' into stable-3.5
* stable-3.4: Paginate no-limit queries Fix pagination to stop querying when there are no more results Avoid fetching more results than needed in paginated index queries Protect query limit effectively against integer overflows Add missing return values in MetricMaker Provide configuration to paginate index queries with increasing size Introduce a SEARCH_AFTER index pagination type ChangeIsVisibleToPredicate: Only create PermissionBackend.WithUser once Move permission filtering for All-Projects from ProjectState to ProjectConfig Change-Id: I11600ad4c11c2c546a23058b1606056d76825cd7 Release-Notes: skip
-rw-r--r--Documentation/config-gerrit.txt57
-rw-r--r--java/com/google/gerrit/index/IndexConfig.java39
-rw-r--r--java/com/google/gerrit/index/PaginationType.java29
-rw-r--r--java/com/google/gerrit/index/QueryOptions.java77
-rw-r--r--java/com/google/gerrit/index/query/AndSource.java79
-rw-r--r--java/com/google/gerrit/index/query/IndexedQuery.java36
-rw-r--r--java/com/google/gerrit/index/query/LazyResultSet.java5
-rw-r--r--java/com/google/gerrit/index/query/ListResultSet.java5
-rw-r--r--java/com/google/gerrit/index/query/Paginated.java4
-rw-r--r--java/com/google/gerrit/index/query/QueryProcessor.java60
-rw-r--r--java/com/google/gerrit/index/query/ResultSet.java2
-rw-r--r--java/com/google/gerrit/index/testing/AbstractFakeIndex.java43
-rw-r--r--java/com/google/gerrit/lucene/AbstractLuceneIndex.java21
-rw-r--r--java/com/google/gerrit/lucene/LuceneChangeIndex.java73
-rw-r--r--java/com/google/gerrit/metrics/MetricMaker.java10
-rw-r--r--java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java2
-rw-r--r--java/com/google/gerrit/server/index/change/IndexedChangeQuery.java27
-rw-r--r--java/com/google/gerrit/server/index/group/IndexedGroupQuery.java4
-rw-r--r--java/com/google/gerrit/server/index/group/StalenessChecker.java2
-rw-r--r--java/com/google/gerrit/server/query/account/AccountQueryProcessor.java4
-rw-r--r--java/com/google/gerrit/server/query/change/AndChangeSource.java10
-rw-r--r--java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java21
-rw-r--r--java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java14
-rw-r--r--java/com/google/gerrit/server/query/group/GroupQueryProcessor.java7
-rw-r--r--java/com/google/gerrit/server/query/project/ProjectQueryProcessor.java7
-rw-r--r--javatests/com/google/gerrit/index/query/LazyDataSourceTest.java9
-rw-r--r--javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java2
-rw-r--r--javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java7
31 files changed, 559 insertions, 118 deletions
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index be801f3e00..fc476ec9c2 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3197,6 +3197,25 @@ advantage of new search features without restarting the server.
+
Defaults to true.
+[[index.paginationType]]index.paginationType::
++
+The pagination type to use when index queries are repeated to
+obtain the next set of results. Supported values are:
++
+* `OFFSET`
++
+Index queries are repeated with a non-zero offset to obtain the
+next set of results.
++
+* `SEARCH_AFTER`
++
+Index queries are repeated using a search-after object. Index
+backends can provide their custom implementations for search-after.
+Note that, `SEARCH_AFTER` does not impact using offsets in Gerrit
+query APIs.
++
+Defaults to `OFFSET`.
+
[[index.maxLimit]]index.maxLimit::
+
Maximum limit to allow for search queries. Requesting results above this
@@ -3215,6 +3234,44 @@ this threshold times the requested limit will result in an error. Set to
+
Defaults to no limit.
+[[index.pageSizeMultiplier]]index.pageSizeMultiplier::
++
+When index queries are repeated to obtain more results, this multiplier
+will be used to determine the limit for the next query. Using a page
+multiplier allows queries to start off small and thus provide good
+latency for queries which may end up only having very few results, and
+then scaling up to have better throughput to handle queries with larger
+result sets without incurring the overhead of making as many queries as
+would be required with a smaller limit. This strategy of using a multiplier
+attempts to create a balance between latency and throughput by dynamically
+adjusting the query size to the number of results being returned by each
+query in the pagination.
++
+The larger the multiplier, the better the throughput on large queries, and
+it also improves latency on large queries by scaling up quickly. However, a
+larger multiplier can hurt latencies a bit by making the "last" query in a
+series longer than needed. The impact of this depends on how much the backend
+latency goes up when specifying a large limit and few results are returned.
+Setting link:#index.maxPageSize[index.maxPageSize] that isn't too large, can
+likely help reduce the impacts of this.
++
+For example, if the limit of the previous query was 500 and pageSizeMultiplier
+is configured to 5, the next query will have a limit of 2500.
++
+Defaults to 1 which effectively turns this feature off.
+
+[[index.maxPageSize]]index.maxPageSize::
++
+Maximum size to allow when index queries are repeated to obtain more results. Note
+that, link:#index.maxLimit[index.maxLimit] will be used to limit page size if it
+is configured to a value lower than maxPageSize.
++
+For example, if the limit of previous query was 500, pageSizeMultiplier is
+configured to 5 and maxPageSize to 2000, the next query will have a limit of
+2000 (instead of 2500).
++
+Defaults to no limit.
+
[[index.maxTerms]]index.maxTerms::
+
Maximum number of leaf terms to allow in a query. Too-large queries may
diff --git a/java/com/google/gerrit/index/IndexConfig.java b/java/com/google/gerrit/index/IndexConfig.java
index 8676fb2f78..c21f32eb5c 100644
--- a/java/com/google/gerrit/index/IndexConfig.java
+++ b/java/com/google/gerrit/index/IndexConfig.java
@@ -30,6 +30,7 @@ import org.eclipse.jgit.lib.Config;
@AutoValue
public abstract class IndexConfig {
private static final int DEFAULT_MAX_TERMS = 1024;
+ private static final int DEFAULT_PAGE_SIZE_MULTIPLIER = 1;
public static IndexConfig createDefault() {
return builder().build();
@@ -40,7 +41,10 @@ public abstract class IndexConfig {
setIfPresent(cfg, "maxLimit", b::maxLimit);
setIfPresent(cfg, "maxPages", b::maxPages);
setIfPresent(cfg, "maxTerms", b::maxTerms);
+ setIfPresent(cfg, "pageSizeMultiplier", b::pageSizeMultiplier);
+ setIfPresent(cfg, "maxPageSize", b::maxPageSize);
setTypeOrDefault(cfg, b::type);
+ setPaginationTypeOrDefault(cfg, b::paginationType);
return b;
}
@@ -56,13 +60,21 @@ public abstract class IndexConfig {
setter.accept(new IndexType(type).toString());
}
+ private static void setPaginationTypeOrDefault(Config cfg, Consumer<PaginationType> setter) {
+ setter.accept(
+ cfg != null ? cfg.getEnum("index", null, "paginationType", PaginationType.OFFSET) : null);
+ }
+
public static Builder builder() {
return new AutoValue_IndexConfig.Builder()
.maxLimit(Integer.MAX_VALUE)
.maxPages(Integer.MAX_VALUE)
.maxTerms(DEFAULT_MAX_TERMS)
+ .pageSizeMultiplier(DEFAULT_PAGE_SIZE_MULTIPLIER)
+ .maxPageSize(Integer.MAX_VALUE)
.type(IndexType.getDefault())
- .separateChangeSubIndexes(false);
+ .separateChangeSubIndexes(false)
+ .paginationType(PaginationType.OFFSET);
}
@AutoValue.Builder
@@ -85,6 +97,12 @@ public abstract class IndexConfig {
public abstract Builder separateChangeSubIndexes(boolean separate);
+ public abstract Builder paginationType(PaginationType type);
+
+ public abstract Builder pageSizeMultiplier(int pageSizeMultiplier);
+
+ public abstract Builder maxPageSize(int maxPageSize);
+
abstract IndexConfig autoBuild();
public IndexConfig build() {
@@ -92,6 +110,8 @@ public abstract class IndexConfig {
checkLimit(cfg.maxLimit(), "maxLimit");
checkLimit(cfg.maxPages(), "maxPages");
checkLimit(cfg.maxTerms(), "maxTerms");
+ checkLimit(cfg.pageSizeMultiplier(), "pageSizeMultiplier");
+ checkLimit(cfg.maxPageSize(), "maxPageSize");
return cfg;
}
}
@@ -124,4 +144,21 @@ public abstract class IndexConfig {
* Returns whether different subsets of changes may be stored in different physical sub-indexes.
*/
public abstract boolean separateChangeSubIndexes();
+
+ /**
+ * Returns pagination type to use when index queries are repeated to obtain the next set of
+ * results.
+ */
+ public abstract PaginationType paginationType();
+
+ /**
+ * Returns multiplier to be used to determine the limit when queries are repeated to obtain the
+ * next set of results.
+ */
+ public abstract int pageSizeMultiplier();
+
+ /**
+ * Returns maximum allowed limit when repeating index queries to obtain the next set of results.
+ */
+ public abstract int maxPageSize();
}
diff --git a/java/com/google/gerrit/index/PaginationType.java b/java/com/google/gerrit/index/PaginationType.java
new file mode 100644
index 0000000000..e7e34fdc0a
--- /dev/null
+++ b/java/com/google/gerrit/index/PaginationType.java
@@ -0,0 +1,29 @@
+// 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;
+
+public enum PaginationType {
+ /** Index queries are restarted at a non-zero offset to obtain the next set of results */
+ OFFSET,
+
+ /**
+ * Index queries are restarted using a search-after object. Supported index backends can provide
+ * their custom implementations for search-after.
+ *
+ * <p>For example, Lucene implementation uses the last doc from the previous search as
+ * search-after object and uses the IndexSearcher.searchAfter API to get the next set of results.
+ */
+ SEARCH_AFTER
+}
diff --git a/java/com/google/gerrit/index/QueryOptions.java b/java/com/google/gerrit/index/QueryOptions.java
index 0401dab067..91c8d1a5e5 100644
--- a/java/com/google/gerrit/index/QueryOptions.java
+++ b/java/com/google/gerrit/index/QueryOptions.java
@@ -19,15 +19,52 @@ import static com.google.common.base.Preconditions.checkArgument;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;
import com.google.common.primitives.Ints;
+import com.google.gerrit.common.Nullable;
import java.util.Set;
import java.util.function.Function;
@AutoValue
public abstract class QueryOptions {
public static QueryOptions create(IndexConfig config, int start, int limit, Set<String> fields) {
+ return create(config, start, null, limit, config.pageSizeMultiplier(), limit, fields);
+ }
+
+ public static QueryOptions create(
+ IndexConfig config, int start, int pageSize, int limit, Set<String> fields) {
+ return create(config, start, null, pageSize, config.pageSizeMultiplier(), limit, fields);
+ }
+
+ public static QueryOptions create(
+ IndexConfig config,
+ int start,
+ int pageSize,
+ int pageSizeMultiplier,
+ int limit,
+ Set<String> fields) {
+ return create(config, start, null, pageSize, pageSizeMultiplier, limit, fields);
+ }
+
+ public static QueryOptions create(
+ IndexConfig config,
+ int start,
+ Object searchAfter,
+ int pageSize,
+ int pageSizeMultiplier,
+ int limit,
+ Set<String> fields) {
checkArgument(start >= 0, "start must be nonnegative: %s", start);
checkArgument(limit > 0, "limit must be positive: %s", limit);
- return new AutoValue_QueryOptions(config, start, limit, ImmutableSet.copyOf(fields));
+ if (searchAfter != null) {
+ checkArgument(start == 0, "start must be 0 when searchAfter is specified: %s", start);
+ }
+ return new AutoValue_QueryOptions(
+ config,
+ start,
+ searchAfter,
+ pageSize,
+ pageSizeMultiplier,
+ limit,
+ ImmutableSet.copyOf(fields));
}
public QueryOptions convertForBackend() {
@@ -36,26 +73,56 @@ public abstract class QueryOptions {
int backendLimit = config().maxLimit();
int limit = Ints.saturatedCast((long) limit() + start());
limit = Math.min(limit, backendLimit);
- return create(config(), 0, limit, fields());
+ int pageSize = Math.min(Ints.saturatedCast((long) pageSize() + start()), backendLimit);
+ return create(config(), 0, null, pageSize, pageSizeMultiplier(), limit, fields());
}
public abstract IndexConfig config();
public abstract int start();
+ @Nullable
+ public abstract Object searchAfter();
+
+ public abstract int pageSize();
+
+ public abstract int pageSizeMultiplier();
+
public abstract int limit();
public abstract ImmutableSet<String> fields();
+ public QueryOptions withPageSize(int pageSize) {
+ return create(
+ config(), start(), searchAfter(), pageSize, pageSizeMultiplier(), limit(), fields());
+ }
+
public QueryOptions withLimit(int newLimit) {
- return create(config(), start(), newLimit, fields());
+ return create(
+ config(), start(), searchAfter(), pageSize(), pageSizeMultiplier(), newLimit, fields());
}
public QueryOptions withStart(int newStart) {
- return create(config(), newStart, limit(), fields());
+ return create(
+ config(), newStart, searchAfter(), pageSize(), pageSizeMultiplier(), limit(), fields());
+ }
+
+ public QueryOptions withSearchAfter(Object newSearchAfter) {
+ // Index search-after APIs don't use 'start', so set it to 0 to be safe. ElasticSearch for
+ // example, expects it to be 0 when using search-after APIs.
+ return create(
+ config(), start(), newSearchAfter, pageSize(), pageSizeMultiplier(), limit(), fields())
+ .withStart(0);
}
public QueryOptions filterFields(Function<QueryOptions, Set<String>> filter) {
- return create(config(), start(), limit(), filter.apply(this));
+ return create(
+ config(),
+ start(),
+ searchAfter(),
+ pageSize(),
+ pageSizeMultiplier(),
+ limit(),
+ filter.apply(this));
}
}
diff --git a/java/com/google/gerrit/index/query/AndSource.java b/java/com/google/gerrit/index/query/AndSource.java
index 538e11b783..de24df05da 100644
--- a/java/com/google/gerrit/index/query/AndSource.java
+++ b/java/com/google/gerrit/index/query/AndSource.java
@@ -20,7 +20,11 @@ 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;
@@ -33,23 +37,30 @@ public class AndSource<T> extends AndPredicate<T>
private final IsVisibleToPredicate<T> isVisibleToPredicate;
private final int start;
private final int cardinality;
+ private final IndexConfig indexConfig;
- public AndSource(Collection<? extends Predicate<T>> that) {
- this(that, null, 0);
+ public AndSource(Collection<? extends Predicate<T>> that, IndexConfig indexConfig) {
+ this(that, null, 0, indexConfig);
}
- public AndSource(Predicate<T> that, IsVisibleToPredicate<T> isVisibleToPredicate) {
- this(that, isVisibleToPredicate, 0);
+ public AndSource(
+ Predicate<T> that, IsVisibleToPredicate<T> isVisibleToPredicate, IndexConfig indexConfig) {
+ this(that, isVisibleToPredicate, 0, indexConfig);
}
- public AndSource(Predicate<T> that, IsVisibleToPredicate<T> isVisibleToPredicate, int start) {
- this(ImmutableList.of(that), isVisibleToPredicate, start);
+ public AndSource(
+ Predicate<T> that,
+ IsVisibleToPredicate<T> isVisibleToPredicate,
+ int start,
+ IndexConfig indexConfig) {
+ this(ImmutableList.of(that), isVisibleToPredicate, start, indexConfig);
}
public AndSource(
Collection<? extends Predicate<T>> that,
IsVisibleToPredicate<T> isVisibleToPredicate,
- int start) {
+ int start,
+ IndexConfig indexConfig) {
super(that);
checkArgument(start >= 0, "negative start: %s", start);
this.isVisibleToPredicate = isVisibleToPredicate;
@@ -71,6 +82,7 @@ public class AndSource<T> extends AndPredicate<T>
}
this.source = s;
this.cardinality = c;
+ this.indexConfig = indexConfig;
}
@Override
@@ -86,37 +98,42 @@ public class AndSource<T> extends AndPredicate<T>
() -> {
List<T> r = new ArrayList<>();
T last = null;
- int nextStart = 0;
- boolean skipped = false;
+ int pageResultSize = 0;
for (T data : buffer(resultSet)) {
if (!isMatchable() || match(data)) {
r.add(data);
- } else {
- skipped = true;
}
last = data;
- nextStart++;
+ pageResultSize++;
}
- if (skipped && last != null && source instanceof Paginated) {
- // If our source is a paginated source and we skipped at
- // least one of its results, we may not have filled the full
- // limit the caller wants. Restart the source and continue.
+ 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;
- while (skipped && r.size() < p.getOptions().limit() + start) {
- skipped = false;
- ResultSet<T> next = p.restart(nextStart);
-
+ 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);
- } else {
- skipped = true;
}
- nextStart++;
+ pageResultSize++;
}
+ nextStart += pageResultSize;
+ searchAfter = next.searchAfter();
}
}
@@ -193,4 +210,20 @@ public class AndSource<T> extends AndPredicate<T>
private DataSource<T> toDataSource(Predicate<T> pred) {
return (DataSource<T>) pred;
}
+
+ 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/IndexedQuery.java b/java/com/google/gerrit/index/query/IndexedQuery.java
index d9e33ea2dc..df14e295cc 100644
--- a/java/com/google/gerrit/index/query/IndexedQuery.java
+++ b/java/com/google/gerrit/index/query/IndexedQuery.java
@@ -87,19 +87,15 @@ public class IndexedQuery<I, T> extends Predicate<T> implements DataSource<T>, P
}
@Override
- public ResultSet<T> restart(int start) {
- opts = opts.withStart(start);
- try {
- source = index.getSource(pred, opts);
- } catch (QueryParseException e) {
- // Don't need to show this exception to the user; the only thing that
- // changed about pred was its start, and any other QPEs that might happen
- // should have already thrown from the constructor.
- throw new StorageException(e);
- }
- // Don't convert start to a limit, since the caller of this method (see
- // AndSource) has calculated the actual number to skip.
- return read();
+ public ResultSet<T> restart(int start, int pageSize) {
+ opts = opts.withStart(start).withPageSize(pageSize);
+ return search();
+ }
+
+ @Override
+ public ResultSet<T> restart(Object searchAfter, int pageSize) {
+ opts = opts.withSearchAfter(searchAfter).withPageSize(pageSize);
+ return search();
}
@Override
@@ -125,4 +121,18 @@ public class IndexedQuery<I, T> extends Predicate<T> implements DataSource<T>, P
public String toString() {
return MoreObjects.toStringHelper("index").add("p", pred).add("opts", opts).toString();
}
+
+ private ResultSet<T> search() {
+ try {
+ source = index.getSource(pred, opts);
+ } catch (QueryParseException e) {
+ // Don't need to show this exception to the user; the only thing that
+ // changed about pred was its start, and any other QPEs that might happen
+ // should have already thrown from the constructor.
+ throw new StorageException(e);
+ }
+ // Don't convert start to a limit, since the caller of this method (see
+ // AndSource) has calculated the actual number to skip.
+ return read();
+ }
}
diff --git a/java/com/google/gerrit/index/query/LazyResultSet.java b/java/com/google/gerrit/index/query/LazyResultSet.java
index f3fab5f5b5..a7d71f003a 100644
--- a/java/com/google/gerrit/index/query/LazyResultSet.java
+++ b/java/com/google/gerrit/index/query/LazyResultSet.java
@@ -53,4 +53,9 @@ public class LazyResultSet<T> implements ResultSet<T> {
@Override
public void close() {}
+
+ @Override
+ public Object searchAfter() {
+ return null;
+ }
}
diff --git a/java/com/google/gerrit/index/query/ListResultSet.java b/java/com/google/gerrit/index/query/ListResultSet.java
index 9d7eadf393..f09fda0cd2 100644
--- a/java/com/google/gerrit/index/query/ListResultSet.java
+++ b/java/com/google/gerrit/index/query/ListResultSet.java
@@ -54,4 +54,9 @@ public class ListResultSet<T> implements ResultSet<T> {
public void close() {
results = null;
}
+
+ @Override
+ public Object searchAfter() {
+ return null;
+ }
}
diff --git a/java/com/google/gerrit/index/query/Paginated.java b/java/com/google/gerrit/index/query/Paginated.java
index e61dd53234..552199093b 100644
--- a/java/com/google/gerrit/index/query/Paginated.java
+++ b/java/com/google/gerrit/index/query/Paginated.java
@@ -19,5 +19,7 @@ import com.google.gerrit.index.QueryOptions;
public interface Paginated<T> {
QueryOptions getOptions();
- ResultSet<T> restart(int start);
+ ResultSet<T> restart(int start, int pageSize);
+
+ ResultSet<T> restart(Object searchAfter, int pageSize);
}
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index 6688f39731..0c9a474497 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -56,7 +56,6 @@ import java.util.stream.IntStream;
*/
public abstract class QueryProcessor<T> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private static final int MAX_LIMIT_BUFFER_MULTIPLIER = 100;
protected static class Metrics {
final Timer1<String> executionTime;
@@ -231,9 +230,10 @@ public abstract class QueryProcessor<T> {
checkSupportedForQueries(q);
int limit = getEffectiveLimit(q);
limits.add(limit);
+ int initialPageSize = getInitialPageSize(q);
- if (limit == getBackendSupportedLimit()) {
- limit--;
+ if (initialPageSize == getBackendSupportedLimit()) {
+ initialPageSize--;
}
int page = (start / limit) + 1;
@@ -242,10 +242,31 @@ public abstract class QueryProcessor<T> {
"Cannot go beyond page " + indexConfig.maxPages() + " of results");
}
- // Always bump limit by 1, even if this results in exceeding the permitted
- // max for this user. The only way to see if there are more entities is to
- // ask for one more result from the query.
- QueryOptions opts = createOptions(indexConfig, start, limit + 1, getRequestedFields());
+ // Always bump initial page size by 1, even if this results in exceeding the
+ // permitted max for this user. The only way to see if there are more entities
+ // is to ask for one more result from the query.
+ try {
+ initialPageSize = Math.addExact(initialPageSize, 1);
+ } catch (ArithmeticException e) {
+ initialPageSize = Integer.MAX_VALUE;
+ }
+
+ // If pageSizeMultiplier is set to 1 (default), update it to 10 for no-limit queries as
+ // it helps improve performance and also prevents no-limit queries from severely degrading
+ // when pagination type is OFFSET.
+ int pageSizeMultiplier = indexConfig.pageSizeMultiplier();
+ if (isNoLimit && pageSizeMultiplier == 1) {
+ pageSizeMultiplier = 10;
+ }
+
+ QueryOptions opts =
+ createOptions(
+ indexConfig,
+ start,
+ initialPageSize,
+ pageSizeMultiplier,
+ limit,
+ getRequestedFields());
logger.atFine().log("Query options: " + opts);
Predicate<T> pred = rewriter.rewrite(q, opts);
if (enforceVisibility) {
@@ -319,8 +340,14 @@ public abstract class QueryProcessor<T> {
}
protected QueryOptions createOptions(
- IndexConfig indexConfig, int start, int limit, Set<String> requestedFields) {
- return QueryOptions.create(indexConfig, start, limit, requestedFields);
+ IndexConfig indexConfig,
+ int start,
+ int pageSize,
+ int pageSizeMultiplier,
+ int limit,
+ Set<String> requestedFields) {
+ return QueryOptions.create(
+ indexConfig, start, pageSize, pageSizeMultiplier, limit, requestedFields);
}
/**
@@ -365,10 +392,7 @@ public abstract class QueryProcessor<T> {
return indexConfig.maxLimit();
}
- private int getEffectiveLimit(Predicate<T> p) {
- if (isNoLimit == true) {
- return getIndexSize() + MAX_LIMIT_BUFFER_MULTIPLIER * getBatchSize();
- }
+ public int getInitialPageSize(Predicate<T> p) {
List<Integer> possibleLimits = new ArrayList<>(4);
possibleLimits.add(getBackendSupportedLimit());
possibleLimits.add(getPermittedLimit());
@@ -383,10 +407,18 @@ public abstract class QueryProcessor<T> {
}
int result = Ordering.natural().min(possibleLimits);
// Should have short-circuited from #query or thrown some other exception before getting here.
- checkState(result > 0, "effective limit should be positive");
+ checkState(result > 0, "effective initial page size 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)
diff --git a/java/com/google/gerrit/index/query/ResultSet.java b/java/com/google/gerrit/index/query/ResultSet.java
index 65fcd4593c..b4bd19e9a7 100644
--- a/java/com/google/gerrit/index/query/ResultSet.java
+++ b/java/com/google/gerrit/index/query/ResultSet.java
@@ -49,4 +49,6 @@ public interface ResultSet<T> extends Iterable<T> {
* the iterator has finished.
*/
void close();
+
+ Object searchAfter();
}
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 6ece45afe8..5713cbae77 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -19,6 +19,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
@@ -52,6 +53,9 @@ import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.Config;
/**
@@ -112,14 +116,24 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
public DataSource<V> getSource(Predicate<V> p, QueryOptions opts) {
List<V> results;
synchronized (indexedDocuments) {
- results =
+ Stream<V> valueStream =
indexedDocuments.values().stream()
.map(doc -> valueFor(doc))
.filter(doc -> p.asMatchable().match(doc))
- .sorted(sortingComparator())
- .skip(opts.start())
- .limit(opts.limit())
- .collect(toImmutableList());
+ .sorted(sortingComparator());
+ if (opts.searchAfter() != null) {
+ ImmutableList<V> valueList = valueStream.collect(toImmutableList());
+ int fromIndex =
+ IntStream.range(0, valueList.size())
+ .filter(i -> keyFor(valueList.get(i)).equals(opts.searchAfter()))
+ .findFirst()
+ .orElse(-1)
+ + 1;
+ int toIndex = Math.min(fromIndex + opts.pageSize(), valueList.size());
+ results = valueList.subList(fromIndex, toIndex);
+ } else {
+ results = valueStream.skip(opts.start()).limit(opts.pageSize()).collect(toImmutableList());
+ }
}
return new DataSource<V>() {
@Override
@@ -129,12 +143,19 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
@Override
public ResultSet<V> read() {
- return new ListResultSet<>(results);
+ return new ListResultSet<>(results) {
+ @Override
+ public Object searchAfter() {
+ @Nullable V last = Iterables.getLast(results, null);
+ return last != null ? keyFor(last) : null;
+ }
+ };
}
@Override
public ResultSet<FieldBundle> readRaw() {
ImmutableList.Builder<FieldBundle> fieldBundles = ImmutableList.builder();
+ K searchAfter = null;
for (V result : results) {
ImmutableListMultimap.Builder<String, Object> fields = ImmutableListMultimap.builder();
for (FieldDef<V, ?> field : getSchema().getFields().values()) {
@@ -148,8 +169,16 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
}
}
fieldBundles.add(new FieldBundle(fields.build()));
+ searchAfter = keyFor(result);
}
- return new ListResultSet<>(fieldBundles.build());
+ ImmutableList<FieldBundle> resultSet = fieldBundles.build();
+ K finalSearchAfter = searchAfter;
+ return new ListResultSet<>(resultSet) {
+ @Override
+ public Object searchAfter() {
+ return finalSearchAfter;
+ }
+ };
}
};
}
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index 1e6ccac89a..6cede89aa5 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -535,20 +535,31 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
private <T> ResultSet<T> readImpl(Function<Document, T> mapper) {
IndexSearcher searcher = null;
+ ScoreDoc scoreDoc = null;
try {
searcher = acquire();
- int realLimit = opts.start() + opts.limit();
- TopFieldDocs docs = searcher.search(query, realLimit, sort);
+ int realLimit = opts.start() + opts.pageSize();
+ TopFieldDocs docs =
+ opts.searchAfter() != null
+ ? searcher.searchAfter(
+ (ScoreDoc) opts.searchAfter(), query, realLimit, sort, false, false)
+ : searcher.search(query, realLimit, sort);
ImmutableList.Builder<T> b = ImmutableList.builderWithExpectedSize(docs.scoreDocs.length);
for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
- ScoreDoc sd = docs.scoreDocs[i];
- Document doc = searcher.doc(sd.doc, opts.fields());
+ scoreDoc = docs.scoreDocs[i];
+ Document doc = searcher.doc(scoreDoc.doc, opts.fields());
T mapperResult = mapper.apply(doc);
if (mapperResult != null) {
b.add(mapperResult);
}
}
- return new ListResultSet<>(b.build());
+ ScoreDoc searchAfter = scoreDoc;
+ return new ListResultSet<T>(b.build()) {
+ @Override
+ public Object searchAfter() {
+ return searchAfter;
+ }
+ };
} catch (IOException e) {
throw new StorageException(e);
} finally {
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index e7d47d0778..23ffdace48 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -64,8 +64,11 @@ import com.google.protobuf.MessageLite;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@@ -360,9 +363,9 @@ public class LuceneChangeIndex implements ChangeIndex {
final Set<String> fields = IndexUtils.changeFields(opts, schema.useLegacyNumericFields());
return new ChangeDataResults(
executor.submit(
- new Callable<List<Document>>() {
+ new Callable<Results>() {
@Override
- public List<Document> call() throws IOException {
+ public Results call() throws IOException {
return doRead(fields);
}
@@ -377,8 +380,12 @@ public class LuceneChangeIndex implements ChangeIndex {
@Override
public ResultSet<FieldBundle> readRaw() {
List<Document> documents;
+ Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex;
+
try {
- documents = doRead(IndexUtils.changeFields(opts, schema.useLegacyNumericFields()));
+ Results r = doRead(IndexUtils.changeFields(opts, schema.useLegacyNumericFields()));
+ documents = r.docs;
+ searchAfterBySubIndex = r.searchAfterBySubIndex;
} catch (IOException e) {
throw new StorageException(e);
}
@@ -399,29 +406,49 @@ public class LuceneChangeIndex implements ChangeIndex {
public void close() {
// Do nothing.
}
+
+ @Override
+ public Object searchAfter() {
+ return searchAfterBySubIndex;
+ }
};
}
- private List<Document> doRead(Set<String> fields) throws IOException {
+ private Results doRead(Set<String> fields) throws IOException {
IndexSearcher[] searchers = new IndexSearcher[indexes.size()];
+ Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex = new HashMap<>();
try {
- int realLimit = opts.start() + opts.limit();
- if (Integer.MAX_VALUE - opts.limit() < opts.start()) {
- realLimit = Integer.MAX_VALUE;
+ int realPageSize = opts.start() + opts.pageSize();
+ if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) {
+ realPageSize = Integer.MAX_VALUE;
}
TopFieldDocs[] hits = new TopFieldDocs[indexes.size()];
for (int i = 0; i < indexes.size(); i++) {
- searchers[i] = indexes.get(i).acquire();
- hits[i] = searchers[i].search(query, realLimit, sort);
+ 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);
+ } else {
+ hits[i] = searchers[i].search(query, realPageSize, sort);
+ }
+ searchAfterBySubIndex.put(
+ subIndex, Iterables.getLast(Arrays.asList(hits[i].scoreDocs), null));
}
- TopDocs docs = TopDocs.merge(sort, realLimit, hits);
+ TopDocs docs = TopDocs.merge(sort, realPageSize, hits);
List<Document> result = new ArrayList<>(docs.scoreDocs.length);
for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
ScoreDoc sd = docs.scoreDocs[i];
result.add(searchers[sd.shardIndex].doc(sd.doc, fields));
}
- return result;
+ return new Results(result, searchAfterBySubIndex);
} finally {
for (int i = 0; i < indexes.size(); i++) {
if (searchers[i] != null) {
@@ -436,11 +463,22 @@ public class LuceneChangeIndex implements ChangeIndex {
}
}
+ private static class Results {
+ List<Document> docs;
+ Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex;
+
+ public Results(List<Document> docs, Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex) {
+ this.docs = docs;
+ this.searchAfterBySubIndex = searchAfterBySubIndex;
+ }
+ }
+
private class ChangeDataResults implements ResultSet<ChangeData> {
- private final Future<List<Document>> future;
+ private final Future<Results> future;
private final Set<String> fields;
+ private Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex;
- ChangeDataResults(Future<List<Document>> future, Set<String> fields) {
+ ChangeDataResults(Future<Results> future, Set<String> fields) {
this.future = future;
this.fields = fields;
}
@@ -453,7 +491,9 @@ public class LuceneChangeIndex implements ChangeIndex {
@Override
public ImmutableList<ChangeData> toList() {
try {
- List<Document> docs = future.get();
+ Results r = future.get();
+ List<Document> docs = r.docs;
+ searchAfterBySubIndex = r.searchAfterBySubIndex;
ImmutableList.Builder<ChangeData> result =
ImmutableList.builderWithExpectedSize(docs.size());
for (Document doc : docs) {
@@ -473,6 +513,11 @@ public class LuceneChangeIndex implements ChangeIndex {
public void close() {
future.cancel(false /* do not interrupt Lucene */);
}
+
+ @Override
+ public Object searchAfter() {
+ return searchAfterBySubIndex;
+ }
}
private static ListMultimap<String, IndexableField> fields(Document doc, Set<String> fields) {
diff --git a/java/com/google/gerrit/metrics/MetricMaker.java b/java/com/google/gerrit/metrics/MetricMaker.java
index 42ec8a09a9..618d421e2e 100644
--- a/java/com/google/gerrit/metrics/MetricMaker.java
+++ b/java/com/google/gerrit/metrics/MetricMaker.java
@@ -78,14 +78,15 @@ public abstract class MetricMaker {
* @param name unique name of the metric.
* @param value only value of the metric.
* @param desc description of the metric.
+ * @return registration handle
*/
- public <V> void newConstantMetric(String name, V value, Description desc) {
+ public <V> RegistrationHandle newConstantMetric(String name, V value, Description desc) {
desc.setConstant();
@SuppressWarnings("unchecked")
Class<V> type = (Class<V>) value.getClass();
CallbackMetric0<V> metric = newCallbackMetric(name, type, desc);
- newTrigger(metric, () -> metric.set(value));
+ return newTrigger(metric, () -> metric.set(value));
}
/**
@@ -107,11 +108,12 @@ public abstract class MetricMaker {
* @param valueClass type of value recorded by the metric.
* @param desc description of the metric.
* @param trigger function to compute the value of the metric.
+ * @return registration handle
*/
- public <V> void newCallbackMetric(
+ public <V> RegistrationHandle newCallbackMetric(
String name, Class<V> valueClass, Description desc, Supplier<V> trigger) {
CallbackMetric0<V> metric = newCallbackMetric(name, valueClass, desc);
- newTrigger(metric, () -> metric.set(trigger.get()));
+ return newTrigger(metric, () -> metric.set(trigger.get()));
}
/**
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
index 63c52977a6..35d167a5ce 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
@@ -281,7 +281,7 @@ public class ChangeIndexRewriter implements IndexRewriter<ChangeData> {
private Predicate<ChangeData> copy(Predicate<ChangeData> in, List<Predicate<ChangeData>> all) {
if (in instanceof AndPredicate) {
- return new AndChangeSource(all);
+ return new AndChangeSource(all, config);
} else if (in instanceof OrPredicate) {
return new OrSource(all);
}
diff --git a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
index be5f803495..1d9bbcd4a3 100644
--- a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
+++ b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
@@ -52,24 +52,40 @@ import java.util.Set;
public class IndexedChangeQuery extends IndexedQuery<Change.Id, ChangeData>
implements ChangeDataSource, Matchable<ChangeData> {
public static QueryOptions oneResult() {
- return createOptions(IndexConfig.createDefault(), 0, 1, ImmutableSet.of());
+ IndexConfig config = IndexConfig.createDefault();
+ return createOptions(config, 0, 1, config.pageSizeMultiplier(), 1, ImmutableSet.of());
}
public static QueryOptions createOptions(
IndexConfig config, int start, int limit, Set<String> fields) {
+ return createOptions(config, start, limit, config.pageSizeMultiplier(), limit, fields);
+ }
+
+ public static QueryOptions createOptions(
+ IndexConfig config,
+ int start,
+ int pageSize,
+ int pageSizeMultiplier,
+ int limit,
+ Set<String> fields) {
// Always include project since it is needed to load the change from NoteDb.
if (!fields.contains(CHANGE.getName()) && !fields.contains(PROJECT.getName())) {
fields = new HashSet<>(fields);
fields.add(PROJECT.getName());
}
- return QueryOptions.create(config, start, limit, fields);
+ return QueryOptions.create(config, start, pageSize, pageSizeMultiplier, limit, fields);
}
@VisibleForTesting
static QueryOptions convertOptions(QueryOptions opts) {
opts = opts.convertForBackend();
return IndexedChangeQuery.createOptions(
- opts.config(), opts.start(), opts.limit(), opts.fields());
+ opts.config(),
+ opts.start(),
+ opts.pageSize(),
+ opts.pageSizeMultiplier(),
+ opts.limit(),
+ opts.fields());
}
private final Map<ChangeData, DataSource<ChangeData>> fromSource;
@@ -110,6 +126,11 @@ public class IndexedChangeQuery extends IndexedQuery<Change.Id, ChangeData>
public void close() {
rs.close();
}
+
+ @Override
+ public Object searchAfter() {
+ return rs.searchAfter();
+ }
};
}
diff --git a/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java b/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java
index 90070b6779..8b0f1f8e93 100644
--- a/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java
+++ b/java/com/google/gerrit/server/index/group/IndexedGroupQuery.java
@@ -34,13 +34,13 @@ public class IndexedGroupQuery extends IndexedQuery<AccountGroup.UUID, InternalG
implements DataSource<InternalGroup> {
public static QueryOptions createOptions(
- IndexConfig config, int start, int limit, Set<String> fields) {
+ IndexConfig config, int start, int pageSize, int limit, Set<String> fields) {
// Always include GroupField.UUID since it is needed to load the group from NoteDb.
if (!fields.contains(GroupField.UUID.getName())) {
fields = new HashSet<>(fields);
fields.add(GroupField.UUID.getName());
}
- return QueryOptions.create(config, start, limit, fields);
+ return QueryOptions.create(config, start, pageSize, limit, fields);
}
public IndexedGroupQuery(
diff --git a/java/com/google/gerrit/server/index/group/StalenessChecker.java b/java/com/google/gerrit/server/index/group/StalenessChecker.java
index 4ce3f5b744..4e992cb659 100644
--- a/java/com/google/gerrit/server/index/group/StalenessChecker.java
+++ b/java/com/google/gerrit/server/index/group/StalenessChecker.java
@@ -66,7 +66,7 @@ public class StalenessChecker {
}
Optional<FieldBundle> result =
- i.getRaw(uuid, IndexedGroupQuery.createOptions(indexConfig, 0, 1, FIELDS));
+ i.getRaw(uuid, IndexedGroupQuery.createOptions(indexConfig, 0, 1, 1, FIELDS));
if (!result.isPresent()) {
// The document is missing in the index.
try (Repository repo = repoManager.openRepository(allUsers)) {
diff --git a/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java b/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java
index e380ef13dd..9893d1a778 100644
--- a/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java
@@ -43,6 +43,7 @@ import com.google.inject.Provider;
public class AccountQueryProcessor extends QueryProcessor<AccountState> {
private final AccountControl.Factory accountControlFactory;
private final Sequences sequences;
+ private final IndexConfig indexConfig;
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -71,12 +72,13 @@ public class AccountQueryProcessor extends QueryProcessor<AccountState> {
() -> limitsFactory.create(userProvider.get()).getQueryLimit());
this.accountControlFactory = accountControlFactory;
this.sequences = sequences;
+ this.indexConfig = indexConfig;
}
@Override
protected Predicate<AccountState> enforceVisibility(Predicate<AccountState> pred) {
return new AndSource<>(
- pred, new AccountIsVisibleToPredicate(accountControlFactory.get()), start);
+ pred, new AccountIsVisibleToPredicate(accountControlFactory.get()), start, indexConfig);
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/AndChangeSource.java b/java/com/google/gerrit/server/query/change/AndChangeSource.java
index 749204f3f1..98cada3d3a 100644
--- a/java/com/google/gerrit/server/query/change/AndChangeSource.java
+++ b/java/com/google/gerrit/server/query/change/AndChangeSource.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.query.AndSource;
import com.google.gerrit.index.query.IsVisibleToPredicate;
import com.google.gerrit.index.query.Predicate;
@@ -22,15 +23,16 @@ import java.util.List;
public class AndChangeSource extends AndSource<ChangeData> implements ChangeDataSource {
- public AndChangeSource(Collection<Predicate<ChangeData>> that) {
- super(that);
+ public AndChangeSource(Collection<Predicate<ChangeData>> that, IndexConfig indexConfig) {
+ super(that, indexConfig);
}
public AndChangeSource(
Predicate<ChangeData> that,
IsVisibleToPredicate<ChangeData> isVisibleToPredicate,
- int start) {
- super(that, isVisibleToPredicate, start);
+ int start,
+ IndexConfig indexConfig) {
+ super(that, isVisibleToPredicate, start, indexConfig);
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index 60587da27b..24042adbf1 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -41,9 +41,8 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
}
protected final CurrentUser user;
- protected final PermissionBackend permissionBackend;
protected final ProjectCache projectCache;
- private final Provider<AnonymousUser> anonymousUserProvider;
+ private final PermissionBackend.WithUser withUser;
@Inject
public ChangeIsVisibleToPredicate(
@@ -53,9 +52,14 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
@Assisted CurrentUser user) {
super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user));
this.user = user;
- this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
- this.anonymousUserProvider = anonymousUserProvider;
+ withUser =
+ user.isIdentifiedUser()
+ ? permissionBackend.absentUser(user.getAccountId())
+ : permissionBackend.user(
+ Optional.of(user)
+ .filter(u -> u instanceof GroupBackedUser || u instanceof InternalUser)
+ .orElseGet(anonymousUserProvider::get));
}
@Override
@@ -74,17 +78,10 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
return false;
}
if (!projectState.get().statePermitsRead()) {
- logger.atFine().log("Filter out change %s of non-reabable project %s", cd, cd.project());
+ logger.atFine().log("Filter out change %s of non-readable project %s", cd, cd.project());
return false;
}
- PermissionBackend.WithUser withUser =
- user.isIdentifiedUser()
- ? permissionBackend.absentUser(user.getAccountId())
- : permissionBackend.user(
- Optional.of(user)
- .filter(u -> u instanceof GroupBackedUser || u instanceof InternalUser)
- .orElseGet(anonymousUserProvider::get));
try {
if (!withUser.change(cd).test(ChangePermission.READ)) {
logger.atFine().log("Filter out non-visisble change: %s", cd);
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 7d02ecd688..7a2dc44a43 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -63,6 +63,7 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
private final List<Extension<ChangePluginDefinedInfoFactory>>
changePluginDefinedInfoFactoriesByPlugin = new ArrayList<>();
private final Sequences sequences;
+ private final IndexConfig indexConfig;
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -93,6 +94,7 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
this.userProvider = userProvider;
this.changeIsVisibleToPredicateFactory = changeIsVisibleToPredicateFactory;
this.sequences = sequences;
+ this.indexConfig = indexConfig;
changePluginDefinedInfoFactories
.entries()
@@ -107,8 +109,14 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
@Override
protected QueryOptions createOptions(
- IndexConfig indexConfig, int start, int limit, Set<String> requestedFields) {
- return IndexedChangeQuery.createOptions(indexConfig, start, limit, requestedFields);
+ IndexConfig indexConfig,
+ int start,
+ int pageSize,
+ int pageSizeMultiplier,
+ int limit,
+ Set<String> requestedFields) {
+ return IndexedChangeQuery.createOptions(
+ indexConfig, start, pageSize, pageSizeMultiplier, limit, requestedFields);
}
@Override
@@ -135,7 +143,7 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
@Override
protected Predicate<ChangeData> enforceVisibility(Predicate<ChangeData> pred) {
return new AndChangeSource(
- pred, changeIsVisibleToPredicateFactory.forUser(userProvider.get()), start);
+ pred, changeIsVisibleToPredicateFactory.forUser(userProvider.get()), start, indexConfig);
}
@Override
diff --git a/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java b/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java
index e5fb036521..c6683faa87 100644
--- a/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java
@@ -44,6 +44,7 @@ public class GroupQueryProcessor extends QueryProcessor<InternalGroup> {
private final Provider<CurrentUser> userProvider;
private final GroupControl.GenericFactory groupControlFactory;
private final Sequences sequences;
+ private final IndexConfig indexConfig;
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -73,12 +74,16 @@ public class GroupQueryProcessor extends QueryProcessor<InternalGroup> {
this.userProvider = userProvider;
this.groupControlFactory = groupControlFactory;
this.sequences = sequences;
+ this.indexConfig = indexConfig;
}
@Override
protected Predicate<InternalGroup> enforceVisibility(Predicate<InternalGroup> pred) {
return new AndSource<>(
- pred, new GroupIsVisibleToPredicate(groupControlFactory, userProvider.get()), start);
+ pred,
+ new GroupIsVisibleToPredicate(groupControlFactory, userProvider.get()),
+ start,
+ indexConfig);
}
@Override
diff --git a/java/com/google/gerrit/server/query/project/ProjectQueryProcessor.java b/java/com/google/gerrit/server/query/project/ProjectQueryProcessor.java
index 5465d6d42e..6dafa92289 100644
--- a/java/com/google/gerrit/server/query/project/ProjectQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/project/ProjectQueryProcessor.java
@@ -46,6 +46,7 @@ public class ProjectQueryProcessor extends QueryProcessor<ProjectData> {
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> userProvider;
private final ProjectCache projectCache;
+ private final IndexConfig indexConfig;
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -75,12 +76,16 @@ public class ProjectQueryProcessor extends QueryProcessor<ProjectData> {
this.permissionBackend = permissionBackend;
this.userProvider = userProvider;
this.projectCache = projectCache;
+ this.indexConfig = indexConfig;
}
@Override
protected Predicate<ProjectData> enforceVisibility(Predicate<ProjectData> pred) {
return new AndSource<>(
- pred, new ProjectIsVisibleToPredicate(permissionBackend, userProvider.get()), start);
+ pred,
+ new ProjectIsVisibleToPredicate(permissionBackend, userProvider.get()),
+ start,
+ indexConfig);
}
@Override
diff --git a/javatests/com/google/gerrit/index/query/LazyDataSourceTest.java b/javatests/com/google/gerrit/index/query/LazyDataSourceTest.java
index 7064f648d4..4105a1d7f5 100644
--- a/javatests/com/google/gerrit/index/query/LazyDataSourceTest.java
+++ b/javatests/com/google/gerrit/index/query/LazyDataSourceTest.java
@@ -17,6 +17,7 @@ package com.google.gerrit.index.query;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeDataSource;
import com.google.gerrit.server.query.change.OrSource;
@@ -88,11 +89,17 @@ public class LazyDataSourceTest {
public void close() {
// No-op
}
+
+ @Override
+ public Object searchAfter() {
+ return null;
+ }
}
@Test
public void andSourceIsLazy() {
- AndSource<ChangeData> and = new AndSource<>(ImmutableList.of(new LazyPredicate()));
+ AndSource<ChangeData> and =
+ new AndSource<>(ImmutableList.of(new LazyPredicate()), IndexConfig.createDefault());
ResultSet<ChangeData> resultSet = and.read();
assertThrows(AssertionError.class, () -> resultSet.toList());
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index e5b2ffb548..e2eaa1df3e 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
@@ -259,7 +259,7 @@ public class ChangeIndexRewriterTest {
@SafeVarargs
private static AndChangeSource andSource(Predicate<ChangeData>... preds) {
- return new AndChangeSource(Arrays.asList(preds));
+ return new AndChangeSource(Arrays.asList(preds), IndexConfig.createDefault());
}
private Predicate<ChangeData> rewrite(Predicate<ChangeData> in) throws QueryParseException {
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java
index 5496f56901..4dde452dc9 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java
@@ -26,4 +26,11 @@ public class FakeQueryChangesLatestIndexVersionTest extends FakeQueryChangesTest
public static Config defaultConfig() {
return IndexConfig.createForFake();
}
+
+ @ConfigSuite.Config
+ public static Config searchAfterPaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "SEARCH_AFTER");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java
index 1610ecacb4..95896dc991 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java
@@ -36,4 +36,11 @@ public class FakeQueryChangesPreviousIndexVersionTest extends FakeQueryChangesTe
IndexConfig.createForFake())
.values());
}
+
+ @ConfigSuite.Config
+ public static Config searchAfterPaginationType() {
+ Config config = againstPreviousIndexVersion();
+ config.setString("index", null, "paginationType", "SEARCH_AFTER");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java
index 52a9170fe1..45879433e6 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java
@@ -23,4 +23,11 @@ public class LuceneQueryChangesLatestIndexVersionTest extends LuceneQueryChanges
public static Config defaultConfig() {
return IndexConfig.createForLucene();
}
+
+ @ConfigSuite.Config
+ public static Config searchAfterPaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "SEARCH_AFTER");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java
index 62483fa0b7..178269730c 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java
@@ -33,4 +33,11 @@ public class LuceneQueryChangesPreviousIndexVersionTest extends LuceneQueryChang
IndexConfig.createForLucene())
.values());
}
+
+ @ConfigSuite.Config
+ public static Config searchAfterPaginationType() {
+ Config config = againstPreviousIndexVersion();
+ config.setString("index", null, "paginationType", "SEARCH_AFTER");
+ return config;
+ }
}