Bind index versions dynamically

We want to support a single index version for reading and multiple
index versions for writing. Wrap these in an IndexCollection with
getSearchIndex and getWriteIndexes methods. Still bind a single
implementation at startup.

Change-Id: Ibc4dbbeba064cde68f2585126d7708db5a2b3410

gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -28,12 +28,15 @@ import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningScheduledExecutorService;
 import com.google.gerrit.extensions.events.LifecycleListener;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.config.GerritServerConfig;
 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;
 import com.google.gerrit.server.index.FieldDef.FillArgs;
 import com.google.gerrit.server.index.FieldType;
+import com.google.gerrit.server.index.IndexCollection;
+import com.google.gerrit.server.index.IndexExecutor;
 import com.google.gerrit.server.index.IndexRewriteImpl;
 import com.google.gerrit.server.index.Schema;
 import com.google.gerrit.server.query.Predicate;
@@ -112,30 +115,38 @@ public class LuceneChangeIndex implements ChangeIndex, LifecycleListener {
 
   private final SitePaths sitePaths;
   private final FillArgs fillArgs;
+  private final IndexCollection indexes;
   private final ExecutorService executor;
-  private final boolean readOnly;
+  private final Schema<ChangeData> schema;
-  private final Schema<ChangeData> fields;
   private final SubIndex openIndex;
   private final SubIndex closedIndex;
+  private final boolean readOnly;
 
-  LuceneChangeIndex(Config cfg, SitePaths sitePaths,
+  LuceneChangeIndex(@GerritServerConfig Config cfg,
-      ListeningScheduledExecutorService executor, FillArgs fillArgs,
+      SitePaths sitePaths,
-      Schema<ChangeData> fields, boolean readOnly) throws IOException {
+      IndexCollection indexes,
+      @IndexExecutor ListeningScheduledExecutorService executor,
+      FillArgs fillArgs,
+      Schema<ChangeData> schema,
+      boolean readOnly) throws IOException {
+    this.indexes = indexes;
     this.sitePaths = sitePaths;
     this.fillArgs = fillArgs;
     this.executor = executor;
+    this.schema = schema;
     this.readOnly = readOnly;
-    this.fields = fields;
 
-    File dir = new File(sitePaths.index_dir, "changes_" + fields.getVersion());
+    File dir = new File(sitePaths.index_dir, "changes_" + schema.getVersion());
     openIndex = new SubIndex(new File(dir, CHANGES_OPEN),
         getIndexWriterConfig(cfg, "changes_open"));
     closedIndex = new SubIndex(new File(dir, CHANGES_CLOSED),
-          getIndexWriterConfig(cfg, "changes_closed"));
+        getIndexWriterConfig(cfg, "changes_closed"));
   }
 
   @Override
   public void start() {
+    indexes.setSearchIndex(this);
+    indexes.addWriteIndex(this);
   }
 
   @Override
@@ -158,6 +169,11 @@ public class LuceneChangeIndex implements ChangeIndex, LifecycleListener {
     }
   }
 
+  @Override
+  public Schema<ChangeData> getSchema() {
+    return schema;
+  }
+
   @SuppressWarnings("unchecked")
   @Override
   public ListenableFuture<Void> insert(ChangeData cd) throws IOException {
@@ -328,7 +344,7 @@ public class LuceneChangeIndex implements ChangeIndex, LifecycleListener {
   private Document toDocument(ChangeData cd) throws IOException {
     try {
       Document result = new Document();
-      for (FieldDef<ChangeData, ?> f : fields.getFields().values()) {
+      for (FieldDef<ChangeData, ?> f : schema.getFields().values()) {
         if (f.isRepeatable()) {
           add(result, f, (Iterable<?>) f.get(cd, fillArgs));
         } else {

gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java

@@ -21,6 +21,7 @@ import com.google.gerrit.server.config.SitePaths;
 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.IndexExecutor;
 import com.google.gerrit.server.index.IndexModule;
 import com.google.inject.Provides;
@@ -60,9 +61,10 @@ public class LuceneIndexModule extends LifecycleModule {
   @Singleton
   public LuceneChangeIndex getChangeIndex(@GerritServerConfig Config cfg,
       SitePaths sitePaths,
+      IndexCollection indexes,
       @IndexExecutor ListeningScheduledExecutorService executor,
       FillArgs fillArgs) throws IOException {
-    return new LuceneChangeIndex(cfg, sitePaths, executor, fillArgs,
+    return new LuceneChangeIndex(cfg, sitePaths, indexes, executor, fillArgs,
         ChangeSchemas.getLatestRelease(), readOnly);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java

@@ -38,6 +38,11 @@ import java.io.IOException;
 public interface ChangeIndex {
   /** Instance indicating secondary index is disabled. */
   public static final ChangeIndex DISABLED = new ChangeIndex() {
+    @Override
+    public Schema<ChangeData> getSchema() {
+      return null;
+    }
+
     @Override
     public ListenableFuture<Void> insert(ChangeData cd) throws IOException {
       return Futures.immediateFuture(null);
@@ -70,6 +75,9 @@ public interface ChangeIndex {
     }
   };
 
+  /** @return the schema version used by this index. */
+  public Schema<ChangeData> getSchema();
+
   /**
    * Insert a change document into the index.
    * <p>

gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java

@@ -41,17 +41,17 @@ public class ChangeIndexerImpl extends ChangeIndexer {
   private static final Logger log =
       LoggerFactory.getLogger(ChangeIndexerImpl.class);
 
-  private final ChangeIndex index;
+  private final IndexCollection indexes;
   private final SchemaFactory<ReviewDb> schemaFactory;
   private final ThreadLocalRequestContext context;
 
   @Inject
   ChangeIndexerImpl(@IndexExecutor ListeningScheduledExecutorService executor,
-      ChangeIndex index,
+      IndexCollection indexes,
       SchemaFactory<ReviewDb> schemaFactory,
       ThreadLocalRequestContext context) {
     super(executor);
-    this.index = index;
+    this.indexes = indexes;
     this.schemaFactory = schemaFactory;
     this.context = context;
   }
@@ -84,7 +84,9 @@ public class ChangeIndexerImpl extends ChangeIndexer {
               throw new OutOfScopeException("No user during ChangeIndexer");
             }
           });
-          index.replace(cd);
+          for (ChangeIndex index : indexes.getWriteIndexes()) {
+            index.replace(cd); // TODO(dborowitz): Parallelize these
+          }
           return null;
         } finally  {
           context.setContext(null);

gerrit-server/src/main/java/com/google/gerrit/server/index/IndexCollection.java

@@ -0,0 +1,69 @@
+// 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.server.index;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.annotation.Nullable;
+
+/** Dynamic pointers to the index versions used for searching and writing. */
+@Singleton
+public class IndexCollection {
+  private final CopyOnWriteArrayList<ChangeIndex> writeIndexes;
+  private final AtomicReference<ChangeIndex> searchIndex;
+
+  @Inject
+  @VisibleForTesting
+  public IndexCollection() {
+    this.writeIndexes = Lists.newCopyOnWriteArrayList();
+    this.searchIndex = new AtomicReference<ChangeIndex>();
+  }
+
+  /**
+   * @return the current search index version, or null if the secondary index is
+   *     disabled.
+   */
+  @Nullable
+  public ChangeIndex getSearchIndex() {
+    return searchIndex.get();
+  }
+
+  public void setSearchIndex(ChangeIndex index) {
+    searchIndex.set(index);
+  }
+
+  public Collection<ChangeIndex> getWriteIndexes() {
+    return Collections.unmodifiableCollection(writeIndexes);
+  }
+
+  public synchronized void addWriteIndex(ChangeIndex index) {
+    int version = index.getSchema().getVersion();
+    for (ChangeIndex i : writeIndexes) {
+      if (i.getSchema().getVersion() == version) {
+        throw new IllegalArgumentException(
+            "Write index version " + version + " already in list");
+      }
+    }
+    writeIndexes.add(index);
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java

@@ -118,15 +118,15 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
     return null;
   }
 
-  private final ChangeIndex index;
+  private final IndexCollection indexes;
   private final Provider<ReviewDb> db;
   private final BasicRewritesImpl basicRewrites;
 
   @Inject
-  IndexRewriteImpl(ChangeIndex index,
+  IndexRewriteImpl(IndexCollection indexes,
       Provider<ReviewDb> db,
       BasicRewritesImpl basicRewrites) {
-    this.index = index;
+    this.indexes = indexes;
     this.db = db;
     this.basicRewrites = basicRewrites;
   }
@@ -235,7 +235,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
 
   private IndexedChangeQuery query(Predicate<ChangeData> p) {
     try {
-      return new IndexedChangeQuery(index, p);
+      return new IndexedChangeQuery(indexes.getSearchIndex(), p);
     } catch (QueryParseException e) {
       throw new IllegalStateException(
           "Failed to convert " + p + " to index predicate", e);

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -33,6 +33,7 @@ import com.google.gerrit.server.account.GroupBackends;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.index.ChangeIndex;
+import com.google.gerrit.server.index.IndexCollection;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCache;
@@ -123,7 +124,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     final PatchListCache patchListCache;
     final GitRepositoryManager repoManager;
     final ProjectCache projectCache;
-    final ChangeIndex index;
+    final IndexCollection indexes;
 
     @Inject
     @VisibleForTesting
@@ -138,7 +139,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         PatchListCache patchListCache,
         GitRepositoryManager repoManager,
         ProjectCache projectCache,
-        ChangeIndex index) {
+        IndexCollection indexes) {
       this.dbProvider = dbProvider;
       this.rewriter = rewriter;
       this.userFactory = userFactory;
@@ -150,7 +151,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
       this.patchListCache = patchListCache;
       this.repoManager = repoManager;
       this.projectCache = projectCache;
-      this.index = index;
+      this.indexes = indexes;
     }
   }
 
@@ -205,10 +206,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
 
   @Operator
   public Predicate<ChangeData> comment(String value) throws QueryParseException {
-    if (args.index == ChangeIndex.DISABLED) {
+    ChangeIndex index = args.indexes.getSearchIndex();
+    if (index == null) {
       throw error("secondary index must be enabled for comment:" + value);
     }
-    return new CommentPredicate(args.dbProvider, args.index, value);
+    return new CommentPredicate(args.dbProvider, index, value);
   }
 
   @Operator
@@ -322,13 +324,13 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
   @Operator
   public Predicate<ChangeData> file(String file) throws QueryParseException {
     if (file.startsWith("^")) {
-      if (allowFileRegex || args.index != ChangeIndex.DISABLED) {
+      if (allowFileRegex || args.indexes.getSearchIndex() != null) {
         return new RegexFilePredicate(args.dbProvider, args.patchListCache, file);
       } else {
         throw error("secondary index must be enabled for file:" + file);
       }
     } else {
-      if (args.index == ChangeIndex.DISABLED) {
+      if (args.indexes.getSearchIndex() == null) {
         throw error("secondary index must be enabled for file:" + file);
       }
       return new EqualsFilePredicate(args.dbProvider, args.patchListCache, file);
@@ -391,11 +393,12 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
 
   @Operator
   public Predicate<ChangeData> message(String text) throws QueryParseException {
-    if (args.index == ChangeIndex.DISABLED) {
+    ChangeIndex index = args.indexes.getSearchIndex();
+    if (index == null) {
       throw error("secondary index must be enabled for message:" + text);
     }
 
-    return new MessagePredicate(args.dbProvider, args.index, text);
+    return new MessagePredicate(args.dbProvider, index, text);
   }
 
   @Operator

gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java

@@ -68,6 +68,11 @@ public class IndexRewriteTest extends TestCase {
       return new Source(p);
     }
 
+    @Override
+    public Schema<ChangeData> getSchema() {
+      throw new UnsupportedOperationException();
+    }
+
     @Override
     public void finishIndex() {
       throw new UnsupportedOperationException();
@@ -102,13 +107,13 @@ public class IndexRewriteTest extends TestCase {
     }
   }
 
-  public static class QueryBuilder extends ChangeQueryBuilder {
+  public class QueryBuilder extends ChangeQueryBuilder {
     QueryBuilder() {
       super(
           new QueryBuilder.Definition<ChangeData, QueryBuilder>(
             QueryBuilder.class),
           new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
-            null, null, null, null, null, null),
+            null, null, null, null, null, indexes),
           null);
     }
 
@@ -122,7 +127,7 @@ public class IndexRewriteTest extends TestCase {
       return predicate("bar", value);
     }
 
-    private static Predicate<ChangeData> predicate(String name, String value) {
+    private Predicate<ChangeData> predicate(String name, String value) {
       return new OperatorPredicate<ChangeData>(name, value) {
         @Override
         public boolean match(ChangeData object) throws OrmException {
@@ -138,6 +143,7 @@ public class IndexRewriteTest extends TestCase {
   }
 
   private DummyIndex index;
+  private IndexCollection indexes;
   private ChangeQueryBuilder queryBuilder;
   private IndexRewriteImpl rewrite;
 
@@ -145,9 +151,11 @@ public class IndexRewriteTest extends TestCase {
   public void setUp() throws Exception {
     super.setUp();
     index = new DummyIndex();
+    indexes = new IndexCollection();
+    indexes.setSearchIndex(index);
     queryBuilder = new QueryBuilder();
     rewrite = new IndexRewriteImpl(
-        index,
+        indexes,
         null,
         new IndexRewriteImpl.BasicRewritesImpl(null));
   }

gerrit-solr/src/main/java/com/google/gerrit/solr/SolrChangeIndex.java

@@ -34,6 +34,7 @@ import com.google.gerrit.server.index.ChangeIndex;
 import com.google.gerrit.server.index.FieldDef;
 import com.google.gerrit.server.index.FieldDef.FillArgs;
 import com.google.gerrit.server.index.FieldType;
+import com.google.gerrit.server.index.IndexCollection;
 import com.google.gerrit.server.index.IndexRewriteImpl;
 import com.google.gerrit.server.index.Schema;
 import com.google.gerrit.server.query.Predicate;
@@ -74,6 +75,7 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
 
   private final FillArgs fillArgs;
   private final SitePaths sitePaths;
+  private final IndexCollection indexes;
   private final CloudSolrServer openIndex;
   private final CloudSolrServer closedIndex;
   private final Schema<ChangeData> schema;
@@ -82,9 +84,11 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
       @GerritServerConfig Config cfg,
       FillArgs fillArgs,
       SitePaths sitePaths,
+      IndexCollection indexes,
       Schema<ChangeData> schema) throws IOException {
     this.fillArgs = fillArgs;
     this.sitePaths = sitePaths;
+    this.indexes = indexes;
     this.schema = schema;
 
     String url = cfg.getString("index", "solr", "url");
@@ -101,7 +105,8 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
 
   @Override
   public void start() {
-    // Do nothing.
+    indexes.setSearchIndex(this);
+    indexes.addWriteIndex(this);
   }
 
   @Override
@@ -110,6 +115,11 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
     closedIndex.shutdown();
   }
 
+  @Override
+  public Schema<ChangeData> getSchema() {
+    return schema;
+  }
+
   @Override
   public ListenableFuture<Void> insert(ChangeData cd) throws IOException {
     String id = cd.getId().toString();

gerrit-solr/src/main/java/com/google/gerrit/solr/SolrIndexModule.java

@@ -15,8 +15,19 @@
 package com.google.gerrit.solr;
 
 import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.SitePaths;
 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.IndexModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.Config;
+
+import java.io.IOException;
 
 public class SolrIndexModule extends LifecycleModule {
   private final boolean checkVersion;
@@ -40,4 +51,14 @@ public class SolrIndexModule extends LifecycleModule {
       listener().to(IndexVersionCheck.class);
     }
   }
+
+  @Provides
+  @Singleton
+  public SolrChangeIndex getChangeIndex(@GerritServerConfig Config cfg,
+      SitePaths sitePaths,
+      IndexCollection indexes,
+      FillArgs fillArgs) throws IOException {
+    return new SolrChangeIndex(cfg, fillArgs, sitePaths, indexes,
+        ChangeSchemas.getLatestRelease());
+  }
 }