diff options
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; + } } |