opendev/gerrit
Code Issues Proposed changes

Provide the group index in slave mode

Browse Source 
With groups in NoteDb, we need the group index to quickly determine the
parent groups of a subgroup or the groups of which a specific account is
a member. These details are necessary for the evaluation of ACLs, and
hence are also required for Gerrit hosts which run in slave mode.

Up to now, we circumvented this issue by still delegating to the
ReviewDb implementation. However, we intend to remove the ReviewDb code
for groups soon and therefore need a working solution for NoteDb.

This change only activates the group index for hosts in slave mode. It
doesn't ensure that the index stays up to date. That aspect will be
handled by a follow-up change.

Change-Id: Idc7bd2bf3aa30f73394da61ea917ae4d47f52c84
This commit is contained in:
Alice Kober-Sotzek
2018-02-08 16:09:50 +01:00
committed by Edwin Kempin
parent cc5b97a26f
commit a264642001
12 changed files with 116 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -343,6 +343,16 @@ 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 {
Iterable<GroupReference> allGroups = groups.getAllGroupReferences(db)::iterator;
for (GroupReference group : allGroups) {
groupIndexer.index(group.getUUID());
}
}
protected static Config submitWholeTopicEnabledConfig() {
@@ -396,10 +406,7 @@ public abstract class AbstractDaemonTest {
// later on. As test indexes are non-permanent, closing an instance and opening another one
// removes all indexed data.
// As a workaround, we simply reindex all available groups here.
Iterable<GroupReference> allGroups = groups.getAllGroupReferences(db)::iterator;
for (GroupReference group : allGroups) {
groupIndexer.index(group.getUUID());
}
reindexAllGroups();
admin = accountCreator.admin();
user = accountCreator.user();

8
java/com/google/gerrit/acceptance/GerritServer.java
View File

@@ -317,7 +317,7 @@ public class GerritServer implements AutoCloseable {
daemon.setEmailModuleForTesting(new FakeEmailSender.Module());
daemon.setAdditionalSysModuleForTesting(testSysModule);
daemon.setEnableSshd(desc.useSsh());
daemon.setSlave(baseConfig.getBoolean("container", "slave", false));
daemon.setSlave(isSlave(baseConfig));
if (desc.memory()) {
checkArgument(additionalArgs.length == 0, "cannot pass args to in-memory server");
@@ -345,7 +345,7 @@ public class GerritServer implements AutoCloseable {
cfg.setBoolean("index", "lucene", "testInmemory", true);
cfg.setString("gitweb", null, "cgi", "");
daemon.setEnableHttpd(desc.httpd());
daemon.setLuceneModule(LuceneIndexModule.singleVersionAllLatest(0));
daemon.setLuceneModule(LuceneIndexModule.singleVersionAllLatest(0, isSlave(baseConfig)));
daemon.setDatabaseForTesting(
ImmutableList.<Module>of(
new InMemoryTestingDatabaseModule(
@@ -354,6 +354,10 @@ public class GerritServer implements AutoCloseable {
return new GerritServer(desc, null, createTestInjector(daemon), daemon, null);
}
private static boolean isSlave(Config baseConfig) {
return baseConfig.getBoolean("container", "slave", false);
}
private static GerritServer startOnDisk(
Description desc,
Path site,

38
java/com/google/gerrit/elasticsearch/ElasticIndexModule.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.elasticsearch;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.IndexModule;
@@ -36,34 +37,41 @@ import org.eclipse.jgit.lib.Config;
public class ElasticIndexModule extends AbstractModule {
public static ElasticIndexModule singleVersionWithExplicitVersions(
Map<String, Integer> versions, int threads) {
return new ElasticIndexModule(versions, threads, false);
Map<String, Integer> versions, int threads, boolean slave) {
return new ElasticIndexModule(versions, threads, false, slave);
}
public static ElasticIndexModule latestVersionWithOnlineUpgrade() {
return new ElasticIndexModule(null, 0, true);
public static ElasticIndexModule latestVersionWithOnlineUpgrade(boolean slave) {
return new ElasticIndexModule(null, 0, true, slave);
}
public static ElasticIndexModule latestVersionWithoutOnlineUpgrade() {
return new ElasticIndexModule(null, 0, false);
public static ElasticIndexModule latestVersionWithoutOnlineUpgrade(boolean slave) {
return new ElasticIndexModule(null, 0, false, slave);
}
private final Map<String, Integer> singleVersions;
private final int threads;
private final boolean onlineUpgrade;
private final boolean slave;
private ElasticIndexModule(
Map<String, Integer> singleVersions, int threads, boolean onlineUpgrade) {
Map<String, Integer> singleVersions, int threads, boolean onlineUpgrade, boolean slave) {
if (singleVersions != null) {
checkArgument(!onlineUpgrade, "online upgrade is incompatible with single version map");
}
this.singleVersions = singleVersions;
this.threads = threads;
this.onlineUpgrade = onlineUpgrade;
this.slave = slave;
}
@Override
protected void configure() {
if (slave) {
bind(AccountIndex.Factory.class).toInstance(ElasticIndexModule::createDummyIndexFactory);
bind(ChangeIndex.Factory.class).toInstance(ElasticIndexModule::createDummyIndexFactory);
bind(ProjectIndex.Factory.class).toInstance(ElasticIndexModule::createDummyIndexFactory);
} else {
install(
new FactoryModuleBuilder()
.implement(AccountIndex.class, ElasticAccountIndex.class)
@@ -72,16 +80,17 @@ public class ElasticIndexModule extends AbstractModule {
new FactoryModuleBuilder()
.implement(ChangeIndex.class, ElasticChangeIndex.class)
.build(ChangeIndex.Factory.class));
install(
new FactoryModuleBuilder()
.implement(GroupIndex.class, ElasticGroupIndex.class)
.build(GroupIndex.Factory.class));
install(
new FactoryModuleBuilder()
.implement(ProjectIndex.class, ElasticProjectIndex.class)
.build(ProjectIndex.Factory.class));
}
install(
new FactoryModuleBuilder()
.implement(GroupIndex.class, ElasticGroupIndex.class)
.build(GroupIndex.Factory.class));
install(new IndexModule(threads));
install(new IndexModule(threads, slave));
if (singleVersions == null) {
install(new MultiVersionModule());
} else {
@@ -89,6 +98,11 @@ public class ElasticIndexModule extends AbstractModule {
}
}
@SuppressWarnings("unused")
private static <T> T createDummyIndexFactory(Schema<?> schema) {
throw new UnsupportedOperationException();
}
@Provides
@Singleton
IndexConfig getIndexConfig(@GerritServerConfig Config cfg) {

4
java/com/google/gerrit/httpd/init/WebAppInitializer.java
View File

@@ -392,9 +392,9 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi
private Module createIndexModule() {
switch (indexType) {
case LUCENE:
return LuceneIndexModule.latestVersionWithOnlineUpgrade();
return LuceneIndexModule.latestVersionWithOnlineUpgrade(false);
case ELASTICSEARCH:
return ElasticIndexModule.latestVersionWithOnlineUpgrade();
return ElasticIndexModule.latestVersionWithOnlineUpgrade(false);
default:
throw new IllegalStateException("unsupported index.type = " + indexType);
}

43
java/com/google/gerrit/lucene/LuceneIndexModule.java
View File

@@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.IndexModule;
@@ -37,21 +38,21 @@ import org.apache.lucene.search.BooleanQuery;
import org.eclipse.jgit.lib.Config;
public class LuceneIndexModule extends AbstractModule {
public static LuceneIndexModule singleVersionAllLatest(int threads) {
return new LuceneIndexModule(ImmutableMap.<String, Integer>of(), threads, false);
public static LuceneIndexModule singleVersionAllLatest(int threads, boolean slave) {
return new LuceneIndexModule(ImmutableMap.of(), threads, false, slave);
}
public static LuceneIndexModule singleVersionWithExplicitVersions(
Map<String, Integer> versions, int threads) {
return new LuceneIndexModule(versions, threads, false);
Map<String, Integer> versions, int threads, boolean slave) {
return new LuceneIndexModule(versions, threads, false, slave);
}
public static LuceneIndexModule latestVersionWithOnlineUpgrade() {
return new LuceneIndexModule(null, 0, true);
public static LuceneIndexModule latestVersionWithOnlineUpgrade(boolean slave) {
return new LuceneIndexModule(null, 0, true, slave);
}
public static LuceneIndexModule latestVersionWithoutOnlineUpgrade() {
return new LuceneIndexModule(null, 0, false);
public static LuceneIndexModule latestVersionWithoutOnlineUpgrade(boolean slave) {
return new LuceneIndexModule(null, 0, false, slave);
}
static boolean isInMemoryTest(Config cfg) {
@@ -61,19 +62,26 @@ public class LuceneIndexModule extends AbstractModule {
private final Map<String, Integer> singleVersions;
private final int threads;
private final boolean onlineUpgrade;
private final boolean slave;
private LuceneIndexModule(
Map<String, Integer> singleVersions, int threads, boolean onlineUpgrade) {
Map<String, Integer> singleVersions, int threads, boolean onlineUpgrade, boolean slave) {
if (singleVersions != null) {
checkArgument(!onlineUpgrade, "online upgrade is incompatible with single version map");
}
this.singleVersions = singleVersions;
this.threads = threads;
this.onlineUpgrade = onlineUpgrade;
this.slave = slave;
}
@Override
protected void configure() {
if (slave) {
bind(AccountIndex.Factory.class).toInstance(LuceneIndexModule::createDummyIndexFactory);
bind(ChangeIndex.Factory.class).toInstance(LuceneIndexModule::createDummyIndexFactory);
bind(ProjectIndex.Factory.class).toInstance(LuceneIndexModule::createDummyIndexFactory);
} else {
install(
new FactoryModuleBuilder()
.implement(AccountIndex.class, LuceneAccountIndex.class)
@@ -82,15 +90,17 @@ public class LuceneIndexModule extends AbstractModule {
new FactoryModuleBuilder()
.implement(ChangeIndex.class, LuceneChangeIndex.class)
.build(ChangeIndex.Factory.class));
install(
new FactoryModuleBuilder()
.implement(GroupIndex.class, LuceneGroupIndex.class)
.build(GroupIndex.Factory.class));
install(
new FactoryModuleBuilder()
.implement(ProjectIndex.class, LuceneProjectIndex.class)
.build(ProjectIndex.Factory.class));
install(new IndexModule(threads));
}
install(
new FactoryModuleBuilder()
.implement(GroupIndex.class, LuceneGroupIndex.class)
.build(GroupIndex.Factory.class));
install(new IndexModule(threads, slave));
if (singleVersions == null) {
install(new MultiVersionModule());
} else {
@@ -98,6 +108,11 @@ public class LuceneIndexModule extends AbstractModule {
}
}
@SuppressWarnings("unused")
private static <T> T createDummyIndexFactory(Schema<?> schema) {
throw new UnsupportedOperationException();
}
@Provides
@Singleton
IndexConfig getIndexConfig(@GerritServerConfig Config cfg) {

14
java/com/google/gerrit/pgm/Daemon.java
View File

@@ -71,7 +71,6 @@ import com.google.gerrit.server.git.LocalMergeSuperSetComputation;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.git.receive.ReceiveCommitsExecutorModule;
import com.google.gerrit.server.index.DummyIndexModule;
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.index.VersionManager;
@@ -334,9 +333,7 @@ public class Daemon extends SiteProgram {
}
cfgInjector = createCfgInjector();
config = cfgInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
if (!slave) {
initIndexType();
}
sysInjector = createSysInjector();
sysInjector.getInstance(PluginGuiceEnvironment.class).setDbCfgInjector(dbInjector, cfgInjector);
manager.add(dbInjector, cfgInjector, sysInjector);
@@ -493,9 +490,6 @@ public class Daemon extends SiteProgram {
}
private Module createIndexModule() {
if (slave) {
return new DummyIndexModule();
}
if (luceneModule != null) {
return luceneModule;
}
@@ -506,12 +500,12 @@ public class Daemon extends SiteProgram {
switch (indexType) {
case LUCENE:
return onlineUpgrade
? LuceneIndexModule.latestVersionWithOnlineUpgrade()
: LuceneIndexModule.latestVersionWithoutOnlineUpgrade();
? LuceneIndexModule.latestVersionWithOnlineUpgrade(slave)
: LuceneIndexModule.latestVersionWithoutOnlineUpgrade(slave);
case ELASTICSEARCH:
return onlineUpgrade
? ElasticIndexModule.latestVersionWithOnlineUpgrade()
: ElasticIndexModule.latestVersionWithoutOnlineUpgrade();
? ElasticIndexModule.latestVersionWithOnlineUpgrade(slave)
: ElasticIndexModule.latestVersionWithoutOnlineUpgrade(slave);
default:
throw new IllegalStateException("unsupported index.type = " + indexType);
}

6
java/com/google/gerrit/pgm/MigrateToNoteDb.java
View File

@@ -203,9 +203,11 @@ public class MigrateToNoteDb extends SiteProgram {
private Module getIndexModule() {
switch (IndexModule.getIndexType(dbInjector)) {
case LUCENE:
return LuceneIndexModule.singleVersionWithExplicitVersions(ImmutableMap.of(), threads);
return LuceneIndexModule.singleVersionWithExplicitVersions(
ImmutableMap.of(), threads, false);
case ELASTICSEARCH:
return ElasticIndexModule.singleVersionWithExplicitVersions(ImmutableMap.of(), threads);
return ElasticIndexModule.singleVersionWithExplicitVersions(
ImmutableMap.of(), threads, false);
default:
throw new IllegalStateException("unsupported index.type");
}

5
java/com/google/gerrit/pgm/Reindex.java
View File

@@ -156,10 +156,11 @@ public class Reindex extends SiteProgram {
Module indexModule;
switch (IndexModule.getIndexType(dbInjector)) {
case LUCENE:
indexModule = LuceneIndexModule.singleVersionWithExplicitVersions(versions, threads);
indexModule = LuceneIndexModule.singleVersionWithExplicitVersions(versions, threads, false);
break;
case ELASTICSEARCH:
indexModule = ElasticIndexModule.singleVersionWithExplicitVersions(versions, threads);
indexModule =
ElasticIndexModule.singleVersionWithExplicitVersions(versions, threads, false);
break;
default:
throw new IllegalStateException("unsupported index.type");

2
java/com/google/gerrit/server/index/DummyIndexModule.java
View File

@@ -59,7 +59,7 @@ public class DummyIndexModule extends AbstractModule {
@Override
protected void configure() {
install(new IndexModule(1));
install(new IndexModule(1, true));
bind(IndexConfig.class).toInstance(IndexConfig.createDefault());
bind(Index.class).toInstance(new DummyChangeIndex());
bind(AccountIndex.Factory.class).toInstance(new DummyAccountIndexFactory());

10
java/com/google/gerrit/server/index/IndexModule.java
View File

@@ -92,9 +92,11 @@ public class IndexModule extends LifecycleModule {
private final ListeningExecutorService interactiveExecutor;
private final ListeningExecutorService batchExecutor;
private final boolean closeExecutorsOnShutdown;
private final boolean slave;
public IndexModule(int threads) {
public IndexModule(int threads, boolean slave) {
this.threads = threads;
this.slave = slave;
this.interactiveExecutor = null;
this.batchExecutor = null;
this.closeExecutorsOnShutdown = true;
@@ -106,6 +108,7 @@ public class IndexModule extends LifecycleModule {
this.interactiveExecutor = interactiveExecutor;
this.batchExecutor = batchExecutor;
this.closeExecutorsOnShutdown = false;
slave = false;
}
@Override
@@ -151,6 +154,11 @@ public class IndexModule extends LifecycleModule {
ChangeIndexDefinition changes,
GroupIndexDefinition groups,
ProjectIndexDefinition projects) {
if (slave) {
// In slave mode, we only have the group index.
return ImmutableList.of(groups);
}
Collection<IndexDefinition<?, ?, ?>> result =
ImmutableList.<IndexDefinition<?, ?, ?>>of(accounts, groups, changes, projects);
Set<String> expected =

6
java/com/google/gerrit/testing/InMemoryModule.java
View File

@@ -281,9 +281,11 @@ public class InMemoryModule extends FactoryModule {
private Module indexModule(String moduleClassName) {
try {
boolean slave = cfg.getBoolean("container", "slave", false);
Class<?> clazz = Class.forName(moduleClassName);
Method m = clazz.getMethod("singleVersionWithExplicitVersions", Map.class, int.class);
return (Module) m.invoke(null, getSingleSchemaVersions(), 0);
Method m =
clazz.getMethod("singleVersionWithExplicitVersions", Map.class, int.class, boolean.class);
return (Module) m.invoke(null, getSingleSchemaVersions(), 0, slave);
} catch (ClassNotFoundException
| SecurityException
| NoSuchMethodException

3
javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
View File

@@ -1220,9 +1220,6 @@ public class GroupsIT extends AbstractDaemonTest {
@Test
@Sandboxed
public void groupsOfUserCanBeListedInSlaveMode() throws Exception {
// TODO(aliceks): Remove this line when we have a group index in slave mode.
assume().that(readGroupsFromNoteDb()).isFalse();
GroupInput groupInput = new GroupInput();
groupInput.name = name("contributors");
groupInput.members = ImmutableList.of(user.username);