Push open/closed subindex into Lucene implementation
Storing open/closed changes in separate indexes is an optimization
specific to the Lucene implementation. Other implementations may want
to do similar splitting, but the unit of splitting is not necessarily
a fully self-contained index.
Leave getPossibleStatus in IndexRewriteImpl, since it may be useful
for other implementations' internal optimizations as well.
Change-Id: Ib2b69f2eb0b1b9246390b9c6c8bc054a9e60836e
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java
index 4fe6def..ab38562 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java
@@ -36,8 +36,8 @@
public class IndexVersionCheck implements LifecycleListener {
public static final Map<String, Integer> SCHEMA_VERSIONS = ImmutableMap.of(
- "changes_open", ChangeField.SCHEMA_VERSION,
- "changes_closed", ChangeField.SCHEMA_VERSION);
+ LuceneChangeIndex.CHANGES_OPEN, ChangeField.SCHEMA_VERSION,
+ LuceneChangeIndex.CHANGES_CLOSED, ChangeField.SCHEMA_VERSION);
public static File gerritIndexConfig(SitePaths sitePaths) {
return new File(sitePaths.index_dir, "gerrit_index.config");
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index ce54b35..f67a76f 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -15,13 +15,18 @@
package com.google.gerrit.lucene;
import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_CHANGE;
+import static com.google.gerrit.server.query.change.IndexRewriteImpl.CLOSED_STATUSES;
+import static com.google.gerrit.server.query.change.IndexRewriteImpl.OPEN_STATUSES;
import static org.apache.lucene.search.BooleanClause.Occur.MUST;
import static org.apache.lucene.search.BooleanClause.Occur.MUST_NOT;
import static org.apache.lucene.search.BooleanClause.Occur.SHOULD;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.ChangeIndex;
import com.google.gerrit.server.index.FieldDef;
@@ -35,38 +40,33 @@
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeDataSource;
+import com.google.gerrit.server.query.change.IndexRewriteImpl;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
-import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.IntField;
import org.apache.lucene.document.StringField;
import org.apache.lucene.index.IndexWriter;
-import org.apache.lucene.index.IndexWriterConfig;
-import org.apache.lucene.index.IndexWriterConfig.OpenMode;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
-import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.SearcherManager;
import org.apache.lucene.search.TermQuery;
-import org.apache.lucene.store.Directory;
-import org.apache.lucene.store.FSDirectory;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.NumericUtils;
import org.apache.lucene.util.Version;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
+import java.util.Set;
/**
* Secondary index implementation using Apache Lucene.
@@ -76,81 +76,86 @@
* though there may be some lag between a committed write and it showing up to
* other threads' searchers.
*/
-public class LuceneChangeIndex implements ChangeIndex {
- private static final Logger log =
- LoggerFactory.getLogger(LuceneChangeIndex.class);
-
+@Singleton
+public class LuceneChangeIndex implements ChangeIndex, LifecycleListener {
public static final Version LUCENE_VERSION = Version.LUCENE_43;
+ public static final String CHANGES_OPEN = "changes_open";
+ public static final String CHANGES_CLOSED = "changes_closed";
private final FillArgs fillArgs;
- private final Directory dir;
- private final IndexWriter writer;
- private final SearcherManager searcherManager;
+ private final SubIndex openIndex;
+ private final SubIndex closedIndex;
- LuceneChangeIndex(File file, FillArgs fillArgs) throws IOException {
+ @Inject
+ LuceneChangeIndex(SitePaths sitePaths, FillArgs fillArgs) throws IOException {
this.fillArgs = fillArgs;
- dir = FSDirectory.open(file);
- IndexWriterConfig writerConfig =
- new IndexWriterConfig(LUCENE_VERSION, new StandardAnalyzer(LUCENE_VERSION));
- writerConfig.setOpenMode(OpenMode.CREATE_OR_APPEND);
- writer = new IndexWriter(dir, writerConfig);
- searcherManager = new SearcherManager(writer, true, null);
+ openIndex = new SubIndex(new File(sitePaths.index_dir, CHANGES_OPEN));
+ closedIndex = new SubIndex(new File(sitePaths.index_dir, CHANGES_CLOSED));
}
- void close() {
- try {
- searcherManager.close();
- } catch (IOException e) {
- log.warn("error closing Lucene searcher", e);
- }
- try {
- writer.close(true);
- } catch (IOException e) {
- log.warn("error closing Lucene writer", e);
- }
- try {
- dir.close();
- } catch (IOException e) {
- log.warn("error closing Lucene directory", e);
- }
+ @Override
+ public void start() {
+ // Do nothing.
+ }
+
+ @Override
+ public void stop() {
+ openIndex.close();
+ closedIndex.close();
}
@Override
public void insert(ChangeData cd) throws IOException {
- writer.addDocument(toDocument(cd));
- commit();
+ Term id = idTerm(cd);
+ Document doc = toDocument(cd);
+ if (cd.getChange().getStatus().isOpen()) {
+ closedIndex.delete(id);
+ openIndex.insert(doc);
+ } else {
+ openIndex.delete(id);
+ closedIndex.insert(doc);
+ }
}
@Override
public void replace(ChangeData cd) throws IOException {
- writer.updateDocument(intTerm(FIELD_CHANGE, cd.getId().get()),
- toDocument(cd));
- commit();
+ Term id = idTerm(cd);
+ Document doc = toDocument(cd);
+ if (cd.getChange().getStatus().isOpen()) {
+ closedIndex.delete(id);
+ openIndex.replace(id, doc);
+ } else {
+ openIndex.delete(id);
+ closedIndex.replace(id, doc);
+ }
}
@Override
public void delete(ChangeData cd) throws IOException {
- writer.deleteDocuments(intTerm(FIELD_CHANGE, cd.getId().get()));
- commit();
+ Term id = idTerm(cd);
+ if (cd.getChange().getStatus().isOpen()) {
+ openIndex.delete(id);
+ } else {
+ closedIndex.delete(id);
+ }
}
@Override
public ChangeDataSource getSource(Predicate<ChangeData> p)
throws QueryParseException {
- return new QuerySource(toQuery(p));
+ Set<Change.Status> statuses = IndexRewriteImpl.getPossibleStatus(p);
+ List<SubIndex> indexes = Lists.newArrayListWithCapacity(2);
+ if (!Sets.intersection(statuses, OPEN_STATUSES).isEmpty()) {
+ indexes.add(openIndex);
+ }
+ if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) {
+ indexes.add(closedIndex);
+ }
+ return new QuerySource(indexes, toQuery(p));
}
- public Directory getDirectory() {
- return dir;
- }
-
- public IndexWriter getWriter() {
- return writer;
- }
-
- private void commit() throws IOException {
- writer.commit();
- searcherManager.maybeRefresh();
+ private Term idTerm(ChangeData cd) {
+ return intTerm(FIELD_CHANGE, cd.getId().get());
}
private Query toQuery(Predicate<ChangeData> p) throws QueryParseException {
@@ -214,9 +219,11 @@
// TODO(dborowitz): Push limit down from predicate tree.
private static final int LIMIT = 1000;
+ private final List<SubIndex> indexes;
private final Query query;
- public QuerySource(Query query) {
+ public QuerySource(List<SubIndex> indexes, Query query) {
+ this.indexes = indexes;
this.query = query;
}
@@ -233,36 +240,28 @@
@Override
public ResultSet<ChangeData> read() throws OrmException {
try {
- IndexSearcher searcher = searcherManager.acquire();
- try {
- ScoreDoc[] docs = searcher.search(query, LIMIT).scoreDocs;
- List<ChangeData> result = Lists.newArrayListWithCapacity(docs.length);
- for (ScoreDoc sd : docs) {
- Document doc = searcher.doc(sd.doc);
- Number v = doc.getField(FIELD_CHANGE).numericValue();
- result.add(new ChangeData(new Change.Id(v.intValue())));
- }
- final List<ChangeData> r = Collections.unmodifiableList(result);
-
- return new ResultSet<ChangeData>() {
- @Override
- public Iterator<ChangeData> iterator() {
- return r.iterator();
- }
-
- @Override
- public List<ChangeData> toList() {
- return r;
- }
-
- @Override
- public void close() {
- // Do nothing.
- }
- };
- } finally {
- searcherManager.release(searcher);
+ List<ChangeData> result =
+ Lists.newArrayListWithExpectedSize(2 * getCardinality());
+ for (SubIndex index : indexes) {
+ result.addAll(index.search(query, LIMIT));
}
+ final List<ChangeData> r = Collections.unmodifiableList(result);
+ return new ResultSet<ChangeData>() {
+ @Override
+ public Iterator<ChangeData> iterator() {
+ return r.iterator();
+ }
+
+ @Override
+ public List<ChangeData> toList() {
+ return r;
+ }
+
+ @Override
+ public void close() {
+ // Do nothing.
+ }
+ };
} catch (IOException e) {
throw new OrmException(e);
}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndexManager.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndexManager.java
deleted file mode 100644
index 1681914..0000000
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndexManager.java
+++ /dev/null
@@ -1,70 +0,0 @@
-// Copyright (C) 2013 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.git;
-
-package com.google.gerrit.lucene;
-
-import com.google.common.base.Throwables;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import com.google.gerrit.extensions.events.LifecycleListener;
-import com.google.gerrit.server.config.SitePaths;
-import com.google.gerrit.server.index.ChangeIndex;
-import com.google.gerrit.server.index.FieldDef.FillArgs;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.concurrent.ExecutionException;
-
-@Singleton
-class LuceneChangeIndexManager implements ChangeIndex.Manager,
- LifecycleListener {
- private final LoadingCache<String, LuceneChangeIndex> indexes;
-
- @Inject
- LuceneChangeIndexManager(final SitePaths sitePaths, final FillArgs fillArgs) {
- indexes = CacheBuilder.newBuilder().build(
- new CacheLoader<String, LuceneChangeIndex>() {
- @Override
- public LuceneChangeIndex load(String key) throws IOException {
- return new LuceneChangeIndex(
- new File(sitePaths.index_dir, key), fillArgs);
- }
- });
- }
-
- @Override
- public void start() {
- // Do nothing.
- }
-
- @Override
- public void stop() {
- for (LuceneChangeIndex index : indexes.asMap().values()) {
- index.close();
- }
- }
-
- @Override
- public LuceneChangeIndex get(String name) throws IOException {
- try {
- return indexes.get(name);
- } catch (ExecutionException e) {
- Throwables.propagateIfInstanceOf(e.getCause(), IOException.class);
- throw new IOException(e);
- }
- }
-}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
index bb92728..440d874 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
@@ -34,8 +34,8 @@
@Override
protected void configure() {
install(new IndexModule(threads));
- bind(ChangeIndex.Manager.class).to(LuceneChangeIndexManager.class);
- listener().to(LuceneChangeIndexManager.class);
+ bind(ChangeIndex.class).to(LuceneChangeIndex.class);
+ listener().to(LuceneChangeIndex.class);
if (checkVersion) {
listener().to(IndexVersionCheck.class);
}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
new file mode 100644
index 0000000..0a48519
--- /dev/null
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
@@ -0,0 +1,115 @@
+// Copyright (C) 2013 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.git;
+
+package com.google.gerrit.lucene;
+
+import static com.google.gerrit.lucene.LuceneChangeIndex.LUCENE_VERSION;
+import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_CHANGE;
+
+import com.google.common.collect.Lists;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.query.change.ChangeData;
+
+import org.apache.lucene.analysis.standard.StandardAnalyzer;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.IndexWriterConfig.OpenMode;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.search.SearcherManager;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+/** Piece of the change index that is implemented as a separate Lucene index. */
+class SubIndex {
+ private static final Logger log =
+ LoggerFactory.getLogger(LuceneChangeIndex.class);
+
+ private final Directory dir;
+ private final IndexWriter writer;
+ private final SearcherManager searcherManager;
+
+ SubIndex(File file) throws IOException {
+ dir = FSDirectory.open(file);
+ IndexWriterConfig writerConfig =
+ new IndexWriterConfig(LUCENE_VERSION, new StandardAnalyzer(LUCENE_VERSION));
+ writerConfig.setOpenMode(OpenMode.CREATE_OR_APPEND);
+ writer = new IndexWriter(dir, writerConfig);
+ searcherManager = new SearcherManager(writer, true, null);
+ }
+
+ void close() {
+ try {
+ searcherManager.close();
+ } catch (IOException e) {
+ log.warn("error closing Lucene searcher", e);
+ }
+ try {
+ writer.close(true);
+ } catch (IOException e) {
+ log.warn("error closing Lucene writer", e);
+ }
+ try {
+ dir.close();
+ } catch (IOException e) {
+ log.warn("error closing Lucene directory", e);
+ }
+ }
+
+ void insert(Document doc) throws IOException {
+ writer.addDocument(doc);
+ commit();
+ }
+
+ void replace(Term term, Document doc) throws IOException {
+ writer.updateDocument(term, doc);
+ commit();
+ }
+
+ void delete(Term term) throws IOException {
+ writer.deleteDocuments(term);
+ commit();
+ }
+
+ List<ChangeData> search(Query query, int limit) throws IOException {
+ IndexSearcher searcher = searcherManager.acquire();
+ try {
+ ScoreDoc[] docs = searcher.search(query, limit).scoreDocs;
+ List<ChangeData> result = Lists.newArrayListWithCapacity(docs.length);
+ for (ScoreDoc sd : docs) {
+ Document doc = searcher.doc(sd.doc);
+ Number v = doc.getField(FIELD_CHANGE).numericValue();
+ result.add(new ChangeData(new Change.Id(v.intValue())));
+ }
+ return Collections.unmodifiableList(result);
+ } finally {
+ searcherManager.release(searcher);
+ }
+ }
+
+ private void commit() throws IOException {
+ writer.commit();
+ searcherManager.maybeRefresh();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
index da2e451..aabe771 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java
@@ -32,38 +32,29 @@
* appropriate.
*/
public interface ChangeIndex {
- public static interface Manager {
- /** Instance indicating secondary index is disabled. */
- public static final Manager DISABLED = new Manager() {
- @Override
- public ChangeIndex get(String name) throws IOException {
- return new ChangeIndex() {
- @Override
- public void insert(ChangeData cd) throws IOException {
- // Do nothing.
- }
+ /** Instance indicating secondary index is disabled. */
+ public static final ChangeIndex DISABLED = new ChangeIndex() {
+ @Override
+ public void insert(ChangeData cd) throws IOException {
+ // Do nothing.
+ }
- @Override
- public void replace(ChangeData cd) throws IOException {
- // Do nothing.
- }
+ @Override
+ public void replace(ChangeData cd) throws IOException {
+ // Do nothing.
+ }
- @Override
- public void delete(ChangeData cd) throws IOException {
- // Do nothing.
- }
+ @Override
+ public void delete(ChangeData cd) throws IOException {
+ // Do nothing.
+ }
- @Override
- public ChangeDataSource getSource(Predicate<ChangeData> p)
- throws QueryParseException {
- throw new UnsupportedOperationException();
- }
- };
- }
- };
-
- ChangeIndex get(String name) throws IOException;
- }
+ @Override
+ public ChangeDataSource getSource(Predicate<ChangeData> p)
+ throws QueryParseException {
+ throw new UnsupportedOperationException();
+ }
+ };
/**
* Insert a change document into the index.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java
index 7a9e869..2e4dee0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java
@@ -37,15 +37,13 @@
LoggerFactory.getLogger(ChangeIndexerImpl.class);
private final ListeningScheduledExecutorService executor;
- private final ChangeIndex openIndex;
- private final ChangeIndex closedIndex;
+ private final ChangeIndex index;
@Inject
ChangeIndexerImpl(@IndexExecutor ListeningScheduledExecutorService executor,
- ChangeIndex.Manager indexManager) throws IOException {
+ ChangeIndex index) throws IOException {
this.executor = executor;
- this.openIndex = indexManager.get("changes_open");
- this.closedIndex = indexManager.get("changes_closed");
+ this.index = index;
}
@Override
@@ -74,13 +72,7 @@
public void run() {
ChangeData cd = new ChangeData(change);
try {
- if (change.getStatus().isOpen()) {
- closedIndex.delete(cd);
- openIndex.replace(cd);
- } else {
- openIndex.delete(cd);
- closedIndex.replace(cd);
- }
+ index.replace(cd);
} catch (IOException e) {
log.error("Error indexing change", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java
index fb7fbe6..1c4ffa8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java
@@ -24,7 +24,7 @@
@Override
protected void configure() {
- bind(ChangeIndex.Manager.class).toInstance(ChangeIndex.Manager.DISABLED);
+ bind(ChangeIndex.class).toInstance(ChangeIndex.DISABLED);
bind(ChangeIndexer.class).toInstance(ChangeIndexer.DISABLED);
bind(IndexRewrite.class).toInstance(IndexRewrite.DISABLED);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java
index f635da6..1cddf82 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java
@@ -14,9 +14,6 @@
package com.google.gerrit.server.index;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
@@ -25,9 +22,6 @@
import com.google.gwtorm.server.ResultSet;
import java.util.Collection;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
/**
* Wrapper combining an {@link IndexPredicate} together with a
@@ -40,68 +34,27 @@
public class PredicateWrapper extends Predicate<ChangeData> implements
ChangeDataSource {
private final Predicate<ChangeData> pred;
- private final List<ChangeDataSource> sources;
+ private final ChangeDataSource source;
- public PredicateWrapper(Predicate<ChangeData> pred, ChangeIndex index)
+ public PredicateWrapper(ChangeIndex index, Predicate<ChangeData> pred)
throws QueryParseException {
- this(pred, ImmutableList.of(index));
- }
-
- public PredicateWrapper(Predicate<ChangeData> pred,
- Collection<ChangeIndex> indexes) throws QueryParseException {
this.pred = pred;
- sources = Lists.newArrayListWithCapacity(indexes.size());
- for (ChangeIndex index : indexes) {
- sources.add(index.getSource(pred));
- }
+ this.source = index.getSource(pred);
}
@Override
public int getCardinality() {
- int n = 0;
- for (ChangeDataSource source : sources) {
- n += source.getCardinality();
- }
- return n;
+ return source.getCardinality();
}
@Override
public boolean hasChange() {
- for (ChangeDataSource source : sources) {
- if (!source.hasChange()) {
- return false;
- }
- }
- return true;
+ return source.hasChange();
}
@Override
public ResultSet<ChangeData> read() throws OrmException {
- final List<ResultSet<ChangeData>> results =
- Lists.newArrayListWithCapacity(sources.size());
- for (ChangeDataSource source : sources) {
- results.add(source.read());
- }
- return new ResultSet<ChangeData>() {
- @Override
- public Iterator<ChangeData> iterator() {
- // TODO(dborowitz): May return duplicates since moving a document
- // between indexes is not atomic.
- return Iterables.concat(results).iterator();
- }
-
- @Override
- public List<ChangeData> toList() {
- return Collections.unmodifiableList(Lists.newArrayList(iterator()));
- }
-
- @Override
- public void close() {
- for (ResultSet<ChangeData> rs : results) {
- rs.close();
- }
- }
- };
+ return source.read();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index d220b83..69207ee 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -114,7 +114,7 @@
final PatchListCache patchListCache;
final GitRepositoryManager repoManager;
final ProjectCache projectCache;
- final ChangeIndex.Manager indexManager;
+ final ChangeIndex index;
@Inject
Arguments(Provider<ReviewDb> dbProvider,
@@ -128,7 +128,7 @@
PatchListCache patchListCache,
GitRepositoryManager repoManager,
ProjectCache projectCache,
- ChangeIndex.Manager indexManager) {
+ ChangeIndex index) {
this.dbProvider = dbProvider;
this.rewriter = rewriter;
this.userFactory = userFactory;
@@ -140,7 +140,7 @@
this.patchListCache = patchListCache;
this.repoManager = repoManager;
this.projectCache = projectCache;
- this.indexManager = indexManager;
+ this.index = index;
}
}
@@ -298,7 +298,7 @@
if (file.startsWith("^")) {
throw error("regular expression not permitted here: file:" + file);
}
- if (args.indexManager == ChangeIndex.Manager.DISABLED) {
+ if (args.index == ChangeIndex.DISABLED) {
throw error("secondary index must be enabled for file:" + file);
}
return new EqualsFilePredicate(args.dbProvider, args.patchListCache, file);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java
index 48f2801..6667054 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.query.change;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.Change;
@@ -28,7 +27,6 @@
import com.google.gerrit.server.query.QueryParseException;
import com.google.inject.Inject;
-import java.io.IOException;
import java.util.BitSet;
import java.util.EnumSet;
import java.util.List;
@@ -36,8 +34,11 @@
/** Rewriter that pushes boolean logic into the secondary index. */
public class IndexRewriteImpl implements IndexRewrite {
- private static final Set<Change.Status> OPEN_STATUSES;
- private static final Set<Change.Status> CLOSED_STATUSES;
+ /** Set of all open change statuses. */
+ public static final Set<Change.Status> OPEN_STATUSES;
+
+ /** Set of all closed change statuses. */
+ public static final Set<Change.Status> CLOSED_STATUSES;
static {
EnumSet<Change.Status> open = EnumSet.noneOf(Change.Status.class);
@@ -53,18 +54,44 @@
CLOSED_STATUSES = Sets.immutableEnumSet(closed);
}
- private final ChangeIndex openIndex;
- private final ChangeIndex closedIndex;
-
- @Inject
- IndexRewriteImpl(ChangeIndex.Manager indexManager) throws IOException {
- this(indexManager.get("changes_open"), indexManager.get("changes_closed"));
+ /**
+ * Get the set of statuses that changes matching the given predicate may have.
+ *
+ * @param in predicate
+ * @return the maximal set of statuses that any changes matching the input
+ * predicates may have, based on examining boolean and
+ * {@link ChangeStatusPredicate}s.
+ */
+ public static EnumSet<Change.Status> getPossibleStatus(Predicate<ChangeData> in) {
+ if (in instanceof ChangeStatusPredicate) {
+ return EnumSet.of(((ChangeStatusPredicate) in).getStatus());
+ } else if (in.getClass() == NotPredicate.class) {
+ return EnumSet.complementOf(getPossibleStatus(in.getChild(0)));
+ } else if (in.getClass() == OrPredicate.class) {
+ EnumSet<Change.Status> s = EnumSet.noneOf(Change.Status.class);
+ for (int i = 0; i < in.getChildCount(); i++) {
+ s.addAll(getPossibleStatus(in.getChild(i)));
+ }
+ return s;
+ } else if (in.getClass() == AndPredicate.class) {
+ EnumSet<Change.Status> s = EnumSet.allOf(Change.Status.class);
+ for (int i = 0; i < in.getChildCount(); i++) {
+ s.retainAll(getPossibleStatus(in.getChild(i)));
+ }
+ return s;
+ } else if (in.getChildCount() == 0) {
+ return EnumSet.allOf(Change.Status.class);
+ } else {
+ throw new IllegalStateException(
+ "Invalid predicate type in change index query: " + in.getClass());
+ }
}
- @VisibleForTesting
- IndexRewriteImpl(ChangeIndex openIndex, ChangeIndex closedIndex) {
- this.openIndex = openIndex;
- this.closedIndex = closedIndex;
+ private final ChangeIndex index;
+
+ @Inject
+ IndexRewriteImpl(ChangeIndex index) {
+ this.index = index;
}
@Override
@@ -181,43 +208,9 @@
}
}
- @VisibleForTesting
- static EnumSet<Change.Status> getPossibleStatus(Predicate<ChangeData> in) {
- if (in instanceof ChangeStatusPredicate) {
- return EnumSet.of(((ChangeStatusPredicate) in).getStatus());
- } else if (in.getClass() == NotPredicate.class) {
- return EnumSet.complementOf(getPossibleStatus(in.getChild(0)));
- } else if (in.getClass() == OrPredicate.class) {
- EnumSet<Change.Status> s = EnumSet.noneOf(Change.Status.class);
- for (int i = 0; i < in.getChildCount(); i++) {
- s.addAll(getPossibleStatus(in.getChild(i)));
- }
- return s;
- } else if (in.getClass() == AndPredicate.class) {
- EnumSet<Change.Status> s = EnumSet.allOf(Change.Status.class);
- for (int i = 0; i < in.getChildCount(); i++) {
- s.retainAll(getPossibleStatus(in.getChild(i)));
- }
- return s;
- } else if (in.getChildCount() == 0) {
- return EnumSet.allOf(Change.Status.class);
- } else {
- throw new IllegalStateException(
- "Invalid predicate type in change index query: " + in.getClass());
- }
- }
-
private PredicateWrapper wrap(Predicate<ChangeData> p) {
try {
- Set<Change.Status> possibleStatus = getPossibleStatus(p);
- List<ChangeIndex> indexes = Lists.newArrayListWithCapacity(2);
- if (!Sets.intersection(possibleStatus, OPEN_STATUSES).isEmpty()) {
- indexes.add(openIndex);
- }
- if (!Sets.intersection(possibleStatus, CLOSED_STATUSES).isEmpty()) {
- indexes.add(closedIndex);
- }
- return new PredicateWrapper(p, indexes);
+ return new PredicateWrapper(index, p);
} catch (QueryParseException e) {
throw new IllegalStateException(
"Failed to convert " + p + " to index predicate", e);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java
index 877d3d6..d7f1a60 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java
@@ -79,22 +79,19 @@
}
}
- private DummyIndex openIndex;
- private DummyIndex closedIndex;
+ private DummyIndex index;
private ChangeQueryBuilder queryBuilder;
private IndexRewrite rewrite;
@Override
public void setUp() throws Exception {
super.setUp();
- openIndex = new DummyIndex();
- closedIndex = new DummyIndex();
+ index = new DummyIndex();
queryBuilder = new ChangeQueryBuilder(
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
null, null, null, null, null, null),
null);
-
- rewrite = new IndexRewriteImpl(openIndex, closedIndex);
+ rewrite = new IndexRewriteImpl(index);
}
public void testIndexPredicate() throws Exception {
@@ -208,7 +205,7 @@
private PredicateWrapper wrap(Predicate<ChangeData> p)
throws QueryParseException {
- return new PredicateWrapper(p, openIndex);
+ return new PredicateWrapper(index, p);
}
private Set<Change.Status> status(String query) throws QueryParseException {
