Add index configuration to specify maximum possible limit

Implementations may not support returning up to Integer.MAX_VALUE
results, so allow this to be specified per implementation. This is
distinct from the configured per-user limits, as it represents a
fundamental limitation of the underlying index backend rather than an
administrative decision.

Encapsulate this in its own class that can be injected into
QueryProcessor, and use this in the limit computation.

Change-Id: I7d1093a2aa3800ff5437c6b1c78b12d73b17db05
This commit is contained in:
Dave Borowitz 2015-01-20 10:34:13 -08:00
parent 959aec4d3e
commit b4de292f8a
6 changed files with 62 additions and 3 deletions

View File

@ -18,6 +18,7 @@ import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.index.ChangeSchemas;
import com.google.gerrit.server.index.IndexCollection;
import com.google.gerrit.server.index.IndexConfig;
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.Schema;
import com.google.gerrit.server.query.change.ChangeData;
@ -43,6 +44,7 @@ public class LuceneIndexModule extends LifecycleModule {
@Override
protected void configure() {
bind(IndexConfig.class).toInstance(IndexConfig.createDefault());
factory(LuceneChangeIndex.Factory.class);
install(new IndexModule(threads));
if (singleVersion == null && base == null) {

View File

@ -53,4 +53,8 @@ public class DummyIndex implements ChangeIndex {
@Override
public void markReady(boolean ready) throws IOException {
}
public int getMaxLimit() {
return Integer.MAX_VALUE;
}
}

View File

@ -21,6 +21,7 @@ public class DummyIndexModule extends AbstractModule {
@Override
protected void configure() {
install(new IndexModule(1));
bind(IndexConfig.class).toInstance(IndexConfig.createDefault());
bind(ChangeIndex.class).toInstance(new DummyIndex());
}
}

View File

@ -0,0 +1,41 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.index;
import static com.google.common.base.Preconditions.checkArgument;
/**
* Implementation-specific configuration for secondary indexes.
* <p>
* Contains configuration that is tied to a specific index implementation but is
* otherwise global, i.e. not tied to a specific {@link ChangeIndex} and schema
* version.
*/
public class IndexConfig {
public static IndexConfig createDefault() {
return new IndexConfig(Integer.MAX_VALUE);
}
private final int maxLimit;
public IndexConfig(int maxLimit) {
checkArgument(maxLimit > 0, "maxLimit must be positive: %s", maxLimit);
this.maxLimit = maxLimit;
}
public int getMaxLimit() {
return maxLimit;
}
}

View File

@ -22,6 +22,7 @@ import com.google.common.collect.Ordering;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.index.IndexConfig;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.Predicate;
@ -39,6 +40,7 @@ public class QueryProcessor {
private final Provider<CurrentUser> userProvider;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeQueryRewriter queryRewriter;
private final IndexConfig indexConfig;
private int limitFromCaller;
private int start;
@ -48,11 +50,13 @@ public class QueryProcessor {
QueryProcessor(Provider<ReviewDb> db,
Provider<CurrentUser> userProvider,
ChangeControl.GenericFactory changeControlFactory,
ChangeQueryRewriter queryRewriter) {
ChangeQueryRewriter queryRewriter,
IndexConfig indexConfig) {
this.db = db;
this.userProvider = userProvider;
this.changeControlFactory = changeControlFactory;
this.queryRewriter = queryRewriter;
this.indexConfig = indexConfig;
}
public QueryProcessor enforceVisibility(boolean enforce) {
@ -125,7 +129,7 @@ public class QueryProcessor {
// 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 changes is to
// ask for one more result from the query.
if (limit == Integer.MAX_VALUE) {
if (limit == getBackendSupportedLimit()) {
limit--;
}
Predicate<ChangeData> s = queryRewriter.rewrite(q, start, limit + 1);
@ -173,8 +177,13 @@ public class QueryProcessor {
return Integer.MAX_VALUE;
}
private int getBackendSupportedLimit() {
return indexConfig.getMaxLimit();
}
private int getEffectiveLimit(Predicate<ChangeData> p) {
List<Integer> possibleLimits = new ArrayList<>(3);
List<Integer> possibleLimits = new ArrayList<>(4);
possibleLimits.add(getBackendSupportedLimit());
possibleLimits.add(getPermittedLimit());
if (limitFromCaller > 0) {
possibleLimits.add(limitFromCaller);

View File

@ -22,6 +22,7 @@ import com.google.gerrit.server.index.ChangeIndex;
import com.google.gerrit.server.index.ChangeSchemas;
import com.google.gerrit.server.index.FieldDef.FillArgs;
import com.google.gerrit.server.index.IndexCollection;
import com.google.gerrit.server.index.IndexConfig;
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Provider;
@ -49,6 +50,7 @@ public class SolrIndexModule extends LifecycleModule {
@Override
protected void configure() {
bind(IndexConfig.class).toInstance(IndexConfig.createDefault());
install(new IndexModule(threads));
bind(ChangeIndex.class).to(SolrChangeIndex.class);
listener().to(SolrChangeIndex.class);