Add config option to reindex all groups on slave startup

If enabled the slave startup is blocked until all stale groups were
reindexed. Enabling this allows to prevent that slaves that were offline
for a longer period of time run with outdated group information until
the first scheduled indexing is done.

E.g. if a slave was offline for 3 weeks and groups are not reindexed on
startup the slave sees group information that is 3 weeks old until the
first scheduled indexing is done. This may be a security issue.

Change-Id: I57ac7ef5c823ec18beef8c19ef159e400b7ada71
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/config-gerrit.txt

@@ -2777,6 +2777,16 @@ reindex] must be performed.
 This section is only used if Gerrit runs in slave mode, otherwise it is
 ignored.
 
+[[index.scheduledIndexer.runOnStartup]]index.scheduledIndexer.runOnStartup::
++
+Whether the scheduled indexer should run once immediately on startup.
+If set to `true` the slave startup is blocked until all stale groups
+were reindexed. Enabling this allows to prevent that slaves that were
+offline for a longer period of time run with outdated group information
+until the first scheduled indexing is done.
++
+Defaults to `true`.
+
 [[index.scheduledIndexer.enabled]]index.scheduledIndexer.enabled::
 +
 Whether the scheduled indexer is enabled. If the scheduled indexer is

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -343,9 +343,6 @@ public abstract class AbstractDaemonTest {
       server.getTestInjector().injectMembers(resetter);
     }
     initSsh();
-    // The server restart threw away all indices. Only reindex all groups as we only have the group
-    // index in slave mode.
-    reindexAllGroups();
   }
 
   private void reindexAllGroups() throws OrmException, IOException, ConfigInvalidException {

java/com/google/gerrit/server/group/PeriodicGroupIndexer.java

@@ -82,6 +82,11 @@ public class PeriodicGroupIndexer implements Runnable {
 
     @Override
     public void start() {
+      boolean runOnStartup = cfg.getBoolean("index", "scheduledIndexer", "runOnStartup", true);
+      if (runOnStartup) {
+        runner.run();
+      }
+
       boolean isEnabled = cfg.getBoolean("index", "scheduledIndexer", "enabled", true);
       if (!isEnabled) {
         log.warn("index.scheduledIndexer is disabled");

javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -1262,14 +1262,15 @@ public class GroupsIT extends AbstractDaemonTest {
         groups.getAllGroupReferences(db).map(GroupReference::getUUID).collect(toList());
     assertThat(expectedGroups.size()).isAtLeast(2);
 
+    // Restart the server as slave, on startup of the slave all groups are indexed.
     restartAsSlave();
 
     GroupIndexedCounter groupIndexedCounter = new GroupIndexedCounter();
     RegistrationHandle groupIndexEventCounterHandle =
         groupIndexedListeners.add(groupIndexedCounter);
     try {
-      // On startup of the slave the test framework ensures that the group index is up-to-date.
-      // Hence running the reindexer doesn't need to reindex any group.
+      // Running the reindexer right after startup should not need to reindex any group since
+      // reindexing was already done on startup.
       slaveGroupIndexer.run();
       groupIndexedCounter.assertNoReindex();
 
@@ -1308,6 +1309,33 @@ public class GroupsIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  @Sandboxed
+  @GerritConfig(name = "index.scheduledIndexer.runOnStartup", value = "false")
+  @GerritConfig(name = "index.scheduledIndexer.enabled", value = "false")
+  @GerritConfig(name = "index.autoReindexIfStale", value = "false")
+  @IgnoreGroupInconsistencies
+  public void disabledReindexGroupsOnStartupSlaveMode() throws Exception {
+    assume().that(readGroupsFromNoteDb()).isTrue();
+
+    List<AccountGroup.UUID> expectedGroups =
+        groups.getAllGroupReferences(db).map(GroupReference::getUUID).collect(toList());
+    assertThat(expectedGroups.size()).isAtLeast(2);
+
+    restartAsSlave();
+
+    GroupIndexedCounter groupIndexedCounter = new GroupIndexedCounter();
+    RegistrationHandle groupIndexEventCounterHandle =
+        groupIndexedListeners.add(groupIndexedCounter);
+    try {
+      // No group indexing happened on startup. All groups should be reindexed now.
+      slaveGroupIndexer.run();
+      groupIndexedCounter.assertReindexOf(expectedGroups);
+    } finally {
+      groupIndexEventCounterHandle.remove();
+    }
+  }
+
   private void assertStaleGroupAndReindex(AccountGroup.UUID groupUuid) throws IOException {
     // Evict group from cache to be sure that we use the index state for staleness checks.
     groupCache.evict(groupUuid);