Add ChangeCache implementation to scan changes from git

The purpose of ChangeCache is to resolve a project name to a list of
changes. This is a natural location for the scanChanges implementation
we added to SiteIndexer. However, most users in a running server will
still just want use the secondary index lookup, as it's faster. So we
extract an interface and provide two implementations.

Change-Id: I66c24b337cdc6aa787f450f8e0b2dfa17b0cdf97

gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNotedb.java

@@ -36,6 +36,7 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MultiProgressMonitor;
 import com.google.gerrit.server.git.MultiProgressMonitor.Task;
@@ -208,6 +209,7 @@ public class RebuildNotedb extends SiteProgram {
       @Override
       public void configure() {
         install(dbInjector.getInstance(BatchProgramModule.class));
+        install(SearchingChangeCacheImpl.module());
         install(new NoteDbModule());
         DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(
             ReindexAfterUpdate.class);

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java

@@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.ScanningChangeCacheImpl;
 import com.google.gerrit.server.index.ChangeIndex;
 import com.google.gerrit.server.index.ChangeSchemas;
 import com.google.gerrit.server.index.IndexCollection;
@@ -68,15 +69,19 @@ public class Reindex extends SiteProgram {
 
   private Injector dbInjector;
   private Injector sysInjector;
+  private Config globalConfig;
   private ChangeIndex index;
 
   @Override
   public int run() throws Exception {
     mustHaveValidSite();
     dbInjector = createDbInjector(MULTI_USER);
+    globalConfig =
+        dbInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
     threads = ThreadLimiter.limitThreads(dbInjector, threads);
     checkNotSlaveMode();
     disableLuceneAutomaticCommit();
+    disableChangeCache();
     if (version == null) {
       version = ChangeSchemas.getLatest().getVersion();
     }
@@ -105,9 +110,7 @@ public class Reindex extends SiteProgram {
   }
 
   private void checkNotSlaveMode() throws Die {
-    Config cfg = dbInjector.getInstance(
+    if (globalConfig.getBoolean("container", "slave", false)) {
-        Key.get(Config.class, GerritServerConfig.class));
-    if (cfg.getBoolean("container", "slave", false)) {
       throw die("Cannot run reindex in slave mode");
     }
   }
@@ -126,20 +129,25 @@ public class Reindex extends SiteProgram {
         throw new IllegalStateException("unsupported index.type");
     }
     modules.add(changeIndexModule);
+    // Scan changes from git instead of relying on the secondary index, as we
+    // will have just deleted the old (possibly corrupt) index.
+    modules.add(ScanningChangeCacheImpl.module());
     modules.add(dbInjector.getInstance(BatchProgramModule.class));
 
     return dbInjector.createChildInjector(modules);
   }
 
   private void disableLuceneAutomaticCommit() {
-    Config cfg =
-        dbInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
     if (IndexModule.getIndexType(dbInjector) == IndexType.LUCENE) {
-      cfg.setLong("index", "changes_open", "commitWithin", -1);
+      globalConfig.setLong("index", "changes_open", "commitWithin", -1);
-      cfg.setLong("index", "changes_closed", "commitWithin", -1);
+      globalConfig.setLong("index", "changes_closed", "commitWithin", -1);
     }
   }
 
+  private void disableChangeCache() {
+    globalConfig.setLong("cache", "changes", "maximumWeight", 0);
+  }
+
   private int indexAll() throws Exception {
     ReviewDb db = sysInjector.getInstance(ReviewDb.class);
     ProgressMonitor pm = new TextProgressMonitor();

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java

@@ -42,7 +42,6 @@ import com.google.gerrit.server.config.DisableReverseDnsLookupProvider;
 import com.google.gerrit.server.config.FactoryModule;
 import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.group.GroupModule;
@@ -128,7 +127,6 @@ public class BatchProgramModule extends FactoryModule {
     install(ProjectCacheImpl.module());
     install(SectionSortCache.module());
     install(ChangeKindCacheImpl.module());
-    install(ChangeCache.module());
     install(MergeabilityCacheImpl.module());
     install(TagCache.module());
     factory(CapabilityControl.Factory.class);

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -73,7 +73,7 @@ import com.google.gerrit.server.change.ChangeKindCacheImpl;
 import com.google.gerrit.server.change.MergeabilityCacheImpl;
 import com.google.gerrit.server.events.EventFactory;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.ChangeCache;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.ChangeMergeQueue;
 import com.google.gerrit.server.git.GarbageCollection;
 import com.google.gerrit.server.git.GitModule;
@@ -163,7 +163,7 @@ public class GerritGlobalModule extends FactoryModule {
     install(authModule);
     install(AccountByEmailCacheImpl.module());
     install(AccountCacheImpl.module());
-    install(ChangeCache.module());
+    install(SearchingChangeCacheImpl.module());
     install(ChangeKindCacheImpl.module());
     install(ConflictsCacheImpl.module());
     install(GroupCacheImpl.module());
@@ -267,7 +267,7 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicSet.setOf(binder(), ProjectDeletedListener.class);
     DynamicSet.setOf(binder(), HeadUpdatedListener.class);
     DynamicSet.setOf(binder(), UsageDataPublishedListener.class);
-    DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ChangeCache.class);
+    DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(SearchingChangeCacheImpl.class);
     DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ReindexAfterUpdate.class);
     DynamicSet.bind(binder(), GitReferenceUpdatedListener.class)
         .to(ProjectConfigEntry.UpdateChecker.class);

gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCache.java

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 The Android Open Source Project
+// 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.
@@ -14,89 +14,11 @@
 
 package com.google.gerrit.server.git;
 
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
-import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.inject.Inject;
-import com.google.inject.Module;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import com.google.inject.TypeLiteral;
-import com.google.inject.name.Named;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.ExecutionException;
 
-@Singleton
+public interface ChangeCache {
-public class ChangeCache implements GitReferenceUpdatedListener {
+  public List<Change> get(Project.NameKey name);
-  private static final Logger log =
-      LoggerFactory.getLogger(ChangeCache.class);
-  private static final String ID_CACHE = "changes";
-
-  public static Module module() {
-    return new CacheModule() {
-      @Override
-      protected void configure() {
-        cache(ID_CACHE,
-            Project.NameKey.class,
-            new TypeLiteral<List<Change>>() {})
-          .maximumWeight(0)
-          .loader(Loader.class);
-      }
-    };
-  }
-
-  private final LoadingCache<Project.NameKey, List<Change>> cache;
-
-  @Inject
-  ChangeCache(@Named(ID_CACHE) LoadingCache<Project.NameKey, List<Change>> cache) {
-    this.cache = cache;
-  }
-
-  List<Change> get(Project.NameKey name) {
-    try {
-      return cache.get(name);
-    } catch (ExecutionException e) {
-      log.warn("Cannot fetch changes for " + name, e);
-      return Collections.emptyList();
-    }
-  }
-
-  @Override
-  public void onGitReferenceUpdated(GitReferenceUpdatedListener.Event event) {
-    if (event.getRefName().startsWith(RefNames.REFS_CHANGES)) {
-      cache.invalidate(new Project.NameKey(event.getProjectName()));
-    }
-  }
-
-  static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
-    private final OneOffRequestContext requestContext;
-    private final Provider<InternalChangeQuery> queryProvider;
-
-    @Inject
-    Loader(OneOffRequestContext requestContext,
-        Provider<InternalChangeQuery> queryProvider) {
-      this.requestContext = requestContext;
-      this.queryProvider = queryProvider;
-    }
-
-    @Override
-    public List<Change> load(Project.NameKey key) throws Exception {
-      try (AutoCloseable ctx = requestContext.open()) {
-        return Collections.unmodifiableList(
-            ChangeData.asChanges(queryProvider.get().byProject(key)));
-      }
-    }
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java

@@ -0,0 +1,123 @@
+// 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.git;
+
+import static com.google.gerrit.server.git.SearchingChangeCacheImpl.ID_CACHE;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.Iterables;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import com.google.inject.name.Named;
+
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+
+@Singleton
+public class ScanningChangeCacheImpl implements ChangeCache {
+  private static final Logger log =
+      LoggerFactory.getLogger(SearchingChangeCacheImpl.class);
+
+  public static Module module() {
+    return new CacheModule() {
+      @Override
+      protected void configure() {
+        bind(ChangeCache.class).to(ScanningChangeCacheImpl.class);
+        cache(ID_CACHE,
+            Project.NameKey.class,
+            new TypeLiteral<List<Change>>() {})
+          .maximumWeight(0)
+          .loader(Loader.class);
+      }
+    };
+  }
+
+  private final LoadingCache<Project.NameKey, List<Change>> cache;
+
+  @Inject
+  ScanningChangeCacheImpl(
+      @Named(ID_CACHE) LoadingCache<Project.NameKey, List<Change>> cache) {
+    this.cache = cache;
+  }
+
+  @Override
+  public List<Change> get(Project.NameKey name) {
+    try {
+      return cache.get(name);
+    } catch (ExecutionException e) {
+      log.warn("Cannot fetch changes for " + name, e);
+      return Collections.emptyList();
+    }
+  }
+
+  static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
+    private final GitRepositoryManager repoManager;
+    private final OneOffRequestContext requestContext;
+
+    @Inject
+    Loader(GitRepositoryManager repoManager,
+        OneOffRequestContext requestContext) {
+      this.repoManager = repoManager;
+      this.requestContext = requestContext;
+    }
+
+    @Override
+    public List<Change> load(Project.NameKey key) throws Exception {
+      Repository repo = repoManager.openRepository(key);
+      try (ManualRequestContext ctx = requestContext.open()) {
+        ReviewDb db = ctx.getReviewDbProvider().get();
+        Map<String, Ref> refs =
+            repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
+        Set<Change.Id> ids = new LinkedHashSet<>();
+        for (Ref r : refs.values()) {
+          Change.Id id = Change.Id.fromRef(r.getName());
+          if (id != null) {
+            ids.add(id);
+          }
+        }
+        List<Change> changes = new ArrayList<>(ids.size());
+         // A batch size of N may overload get(Iterable), so use something smaller,
+         // but still >1.
+        for (List<Change.Id> batch : Iterables.partition(ids, 30)) {
+          Iterables.addAll(changes, db.changes().get(batch));
+        }
+        return changes;
+      } finally {
+        repo.close();
+      }
+    }
+
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java

@@ -0,0 +1,106 @@
+// Copyright (C) 2012 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;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import com.google.inject.name.Named;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+
+@Singleton
+public class SearchingChangeCacheImpl
+    implements ChangeCache, GitReferenceUpdatedListener {
+  private static final Logger log =
+      LoggerFactory.getLogger(SearchingChangeCacheImpl.class);
+  static final String ID_CACHE = "changes";
+
+  public static Module module() {
+    return new CacheModule() {
+      @Override
+      protected void configure() {
+        bind(ChangeCache.class).to(SearchingChangeCacheImpl.class);
+        cache(ID_CACHE,
+            Project.NameKey.class,
+            new TypeLiteral<List<Change>>() {})
+          .maximumWeight(0)
+          .loader(Loader.class);
+      }
+    };
+  }
+
+  private final LoadingCache<Project.NameKey, List<Change>> cache;
+
+  @Inject
+  SearchingChangeCacheImpl(
+      @Named(ID_CACHE) LoadingCache<Project.NameKey, List<Change>> cache) {
+    this.cache = cache;
+  }
+
+  @Override
+  public List<Change> get(Project.NameKey name) {
+    try {
+      return cache.get(name);
+    } catch (ExecutionException e) {
+      log.warn("Cannot fetch changes for " + name, e);
+      return Collections.emptyList();
+    }
+  }
+
+  @Override
+  public void onGitReferenceUpdated(GitReferenceUpdatedListener.Event event) {
+    if (event.getRefName().startsWith(RefNames.REFS_CHANGES)) {
+      cache.invalidate(new Project.NameKey(event.getProjectName()));
+    }
+  }
+
+  static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
+    private final OneOffRequestContext requestContext;
+    private final Provider<InternalChangeQuery> queryProvider;
+
+    @Inject
+    Loader(OneOffRequestContext requestContext,
+        Provider<InternalChangeQuery> queryProvider) {
+      this.requestContext = requestContext;
+      this.queryProvider = queryProvider;
+    }
+
+    @Override
+    public List<Change> load(Project.NameKey key) throws Exception {
+      try (AutoCloseable ctx = requestContext.open()) {
+        return Collections.unmodifiableList(
+            ChangeData.asChanges(queryProvider.get().byProject(key)));
+      }
+    }
+  }
+}

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

@@ -21,7 +21,6 @@ import static org.eclipse.jgit.lib.RefDatabase.ALL;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
@@ -32,16 +31,15 @@ import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.MultiProgressMonitor;
 import com.google.gerrit.server.git.MultiProgressMonitor.Task;
 import com.google.gerrit.server.patch.PatchListLoader;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 
@@ -68,11 +66,9 @@ import org.slf4j.LoggerFactory;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintWriter;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -116,6 +112,7 @@ public class SiteIndexer {
   }
 
   private final SchemaFactory<ReviewDb> schemaFactory;
+  private final ChangeCache changeCache;
   private final ChangeData.Factory changeDataFactory;
   private final GitRepositoryManager repoManager;
   private final ListeningExecutorService executor;
@@ -129,12 +126,14 @@ public class SiteIndexer {
 
   @Inject
   SiteIndexer(SchemaFactory<ReviewDb> schemaFactory,
+      ChangeCache changeCache,
       ChangeData.Factory changeDataFactory,
       GitRepositoryManager repoManager,
       @IndexExecutor(BATCH) ListeningExecutorService executor,
       ChangeIndexer.Factory indexerFactory,
       @GerritServerConfig Config config) {
     this.schemaFactory = schemaFactory;
+    this.changeCache = changeCache;
     this.changeDataFactory = changeDataFactory;
     this.repoManager = repoManager;
     this.executor = executor;
@@ -242,7 +241,7 @@ public class SiteIndexer {
           repo = repoManager.openRepository(project);
           Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL);
           db = schemaFactory.open();
-          for (Change c : scanChanges(db, repo)) {
+          for (Change c : changeCache.get(project)) {
             Ref r = refs.get(c.currentPatchSetId().toRefName());
             if (r != null) {
               byId.put(r.getObjectId(), changeDataFactory.create(db, c));
@@ -273,26 +272,6 @@ public class SiteIndexer {
     };
   }
 
-  private static List<Change> scanChanges(ReviewDb db, Repository repo)
-      throws OrmException, IOException {
-    Map<String, Ref> refs =
-        repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
-    Set<Change.Id> ids = new LinkedHashSet<>();
-    for (Ref r : refs.values()) {
-      Change.Id id = Change.Id.fromRef(r.getName());
-      if (id != null) {
-        ids.add(id);
-      }
-    }
-    List<Change> changes = new ArrayList<>(ids.size());
-     // A batch size of N may overload get(Iterable), so use something smaller,
-     // but still >1.
-    for (List<Change.Id> batch : Iterables.partition(ids, 30)) {
-      Iterables.addAll(changes, db.changes().get(batch));
-    }
-    return changes;
-  }
-
   private static class ProjectIndexer implements Callable<Void> {
     private final ChangeIndexer indexer;
     private final ThreeWayMergeStrategy mergeStrategy;