diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2021-11-23 00:26:38 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2021-11-23 00:26:38 +0000 |
commit | c98514f9687ea1fa28ccb64aad251a51ce64bc30 (patch) | |
tree | d4f429eada2f8aec9bd7d8d8ca0ba1683294cf1b | |
parent | 7d823977c6e3f634e42d3672834ba61eca37ea31 (diff) | |
parent | fff179df4c52837e77e209a5763562e254e64181 (diff) |
Merge "Merge branch 'stable-3.4' into stable-3.5" into stable-3.5
62 files changed, 726 insertions, 411 deletions
@@ -13,10 +13,6 @@ build --incompatible_strict_action_env build --announce_rc -# Workaround Bazel worker crash (remove after upgrading to 4.1.0) -# https://github.com/bazelbuild/bazel/issues/13333 -build --experimental_worker_multiplex=false - test --build_tests_only test --test_output=errors test --java_toolchain=//tools:error_prone_warnings_toolchain_java11 diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt index 1f28561614..7c4ec18ee9 100644 --- a/Documentation/dev-bazel.txt +++ b/Documentation/dev-bazel.txt @@ -268,7 +268,15 @@ The html files will be bundled into `searchfree.zip` in this location: bazel-bin/Documentation/searchfree.zip ---- -To build the executable WAR with the documentation included: +To generate HTML files skipping the zip archiving: + +---- + bazel build Documentation +---- + +And open `bazel-bin/Documentation/index.html`. + +To build the Gerrit executable WAR with the documentation included: ---- bazel build withdocs diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index a3b5e8d190..c68a4fc179 100644 --- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -1158,7 +1158,7 @@ public abstract class AbstractDaemonTest { ProjectConfig config = projectConfigFactory.read(md); config.updateProject(p -> p.setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, value)); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); } } @@ -1167,7 +1167,7 @@ public abstract class AbstractDaemonTest { ProjectConfig config = projectConfigFactory.read(md); config.updateProject(p -> p.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value)); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); } } @@ -1681,7 +1681,7 @@ public abstract class AbstractDaemonTest { projectConfig.commit(metaDataUpdate); metaDataUpdate.close(); metaDataUpdate = null; - projectCache.evict(projectConfig.getProject()); + projectCache.evictAndReindex(projectConfig.getProject()); } @Override diff --git a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java index 271d15c176..7bd0c73013 100644 --- a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java +++ b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java @@ -45,6 +45,11 @@ public class DisabledAccountIndex implements AccountIndex { } @Override + public void insert(AccountState obj) { + throw new UnsupportedOperationException("AccountIndex is disabled"); + } + + @Override public void replace(AccountState obj) { throw new UnsupportedOperationException("AccountIndex is disabled"); } diff --git a/java/com/google/gerrit/acceptance/DisabledChangeIndex.java b/java/com/google/gerrit/acceptance/DisabledChangeIndex.java index 34f72f5ccf..7671ad4068 100644 --- a/java/com/google/gerrit/acceptance/DisabledChangeIndex.java +++ b/java/com/google/gerrit/acceptance/DisabledChangeIndex.java @@ -52,6 +52,11 @@ public class DisabledChangeIndex implements ChangeIndex { } @Override + public void insert(ChangeData obj) { + throw new UnsupportedOperationException("ChangeIndex is disabled"); + } + + @Override public void replace(ChangeData obj) { throw new UnsupportedOperationException("ChangeIndex is disabled"); } diff --git a/java/com/google/gerrit/acceptance/DisabledProjectIndex.java b/java/com/google/gerrit/acceptance/DisabledProjectIndex.java index ed119ff3bc..2e3dd906b6 100644 --- a/java/com/google/gerrit/acceptance/DisabledProjectIndex.java +++ b/java/com/google/gerrit/acceptance/DisabledProjectIndex.java @@ -50,6 +50,11 @@ public class DisabledProjectIndex implements ProjectIndex { } @Override + public void insert(ProjectData obj) { + throw new UnsupportedOperationException("ProjectIndex is disabled"); + } + + @Override public void replace(ProjectData obj) { throw new UnsupportedOperationException("ProjectIndex is disabled"); } diff --git a/java/com/google/gerrit/acceptance/ProjectResetter.java b/java/com/google/gerrit/acceptance/ProjectResetter.java index d8853033d2..46f7496e82 100644 --- a/java/com/google/gerrit/acceptance/ProjectResetter.java +++ b/java/com/google/gerrit/acceptance/ProjectResetter.java @@ -307,7 +307,7 @@ public class ProjectResetter implements AutoCloseable { Sets.union( projectsWithConfigChanges(restoredRefsByProject), projectsWithConfigChanges(deletedRefsByProject))) { - projectCache.evict(project); + projectCache.evictAndReindex(project); } } diff --git a/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java b/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java index e943519d19..f7a0669739 100644 --- a/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java +++ b/java/com/google/gerrit/acceptance/ReadOnlyChangeIndex.java @@ -45,6 +45,11 @@ class ReadOnlyChangeIndex implements ChangeIndex { } @Override + public void insert(ChangeData obj) { + // do nothing + } + + @Override public void replace(ChangeData obj) { // do nothing } diff --git a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java index 16dca66b23..18da4b3aa0 100644 --- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java +++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java @@ -147,7 +147,7 @@ public class ProjectOperationsImpl implements ProjectOperations { setExclusiveGroupPermissions(projectConfig, projectUpdate.exclusiveGroupPermissions()); projectConfig.commit(metaDataUpdate); } - projectCache.evict(nameKey); + projectCache.evictAndReindex(nameKey); } private void removePermissions( @@ -292,7 +292,7 @@ public class ProjectOperationsImpl implements ProjectOperations { setConfig(projectConfig); try { - projectCache.evict(nameKey); + projectCache.evictAndReindex(nameKey); } catch (Exception e) { // Evicting the project from the cache, also triggers a reindex of the project. // The reindex step fails if the project config is invalid. That's fine, since it was our @@ -310,7 +310,7 @@ public class ProjectOperationsImpl implements ProjectOperations { testProjectInvalidation.projectConfigUpdater().forEach(c -> c.accept(projectConfig)); setConfig(projectConfig); try { - projectCache.evict(nameKey); + projectCache.evictAndReindex(nameKey); } catch (Exception e) { // Evicting the project from the cache, also triggers a reindex of the project. // The reindex step fails if the project config is invalid. That's fine, since it was our diff --git a/java/com/google/gerrit/index/Index.java b/java/com/google/gerrit/index/Index.java index ead302dcb8..cc3117d8bd 100644 --- a/java/com/google/gerrit/index/Index.java +++ b/java/com/google/gerrit/index/Index.java @@ -40,6 +40,16 @@ public interface Index<K, V> { void close(); /** + * Insert a document into the index. + * + * <p>Results may not be immediately visible to searchers, but should be visible within a + * reasonable amount of time. + * + * @param obj document object + */ + void insert(V obj); + + /** * Update a document in the index. * * <p>Semantically equivalent to deleting the document and reinserting it with new field values. A diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java index b727e96a9e..6ece45afe8 100644 --- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java +++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java @@ -236,6 +236,9 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> { } return cd; } + + @Override + public void insert(ChangeData obj) {} } /** Fake implementation of {@link AccountIndex} where all filtering happens in-memory. */ @@ -265,6 +268,9 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> { protected Comparator<AccountState> sortingComparator() { return Comparator.comparing(a -> a.account().id().get()); } + + @Override + public void insert(AccountState obj) {} } /** Fake implementation of {@link GroupIndex} where all filtering happens in-memory. */ @@ -295,6 +301,9 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> { protected Comparator<InternalGroup> sortingComparator() { return Comparator.comparing(g -> g.getId().get()); } + + @Override + public void insert(InternalGroup obj) {} } /** Fake implementation of {@link ProjectIndex} where all filtering happens in-memory. */ @@ -324,5 +333,8 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> { protected Comparator<ProjectData> sortingComparator() { return Comparator.comparing(p -> p.getProject().getName()); } + + @Override + public void insert(ProjectData obj) {} } } diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java index 36f800f5bd..475dac4dad 100644 --- a/java/com/google/gerrit/lucene/ChangeSubIndex.java +++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java @@ -90,6 +90,11 @@ public class ChangeSubIndex extends AbstractLuceneIndex<Change.Id, ChangeData> } @Override + public void insert(ChangeData obj) { + throw new UnsupportedOperationException("don't use ChangeSubIndex directly"); + } + + @Override public void replace(ChangeData obj) { throw new UnsupportedOperationException("don't use ChangeSubIndex directly"); } diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java index 87b7ccef6b..c4a524060d 100644 --- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java +++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java @@ -144,6 +144,15 @@ public class LuceneAccountIndex extends AbstractLuceneIndex<Account.Id, AccountS } @Override + public void insert(AccountState as) { + try { + insert(toDocument(as)).get(); + } catch (ExecutionException | InterruptedException e) { + throw new StorageException(e); + } + } + + @Override public void delete(Account.Id key) { try { delete(idTerm(getSchema().useLegacyNumericFields(), key)).get(); diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java index cc58c65a29..e986d40c52 100644 --- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -253,6 +253,24 @@ public class LuceneChangeIndex implements ChangeIndex { } @Override + public void insert(ChangeData cd) { + @SuppressWarnings("unused") // TODO: Why do we need to find the lucene id? + Term id = LuceneChangeIndex.idTerm(idTerm, idField, cd); + // toDocument is essentially static and doesn't depend on the specific + // sub-index, so just pick one. + Document doc = openIndex.toDocument(cd); + try { + if (cd.change().isNew()) { + openIndex.insert(doc).get(); + } else { + closedIndex.insert(doc).get(); + } + } catch (ExecutionException | InterruptedException e) { + throw new StorageException(e); + } + } + + @Override public void delete(Change.Id changeId) { Term id = LuceneChangeIndex.idTerm(idTerm, idField, changeId); try { diff --git a/java/com/google/gerrit/lucene/LuceneGroupIndex.java b/java/com/google/gerrit/lucene/LuceneGroupIndex.java index 816739bf1c..f7a2248e67 100644 --- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java +++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java @@ -125,6 +125,15 @@ public class LuceneGroupIndex extends AbstractLuceneIndex<AccountGroup.UUID, Int } @Override + public void insert(InternalGroup group) { + try { + insert(toDocument(group)).get(); + } catch (ExecutionException | InterruptedException e) { + throw new StorageException(e); + } + } + + @Override public void delete(AccountGroup.UUID key) { try { delete(idTerm(key)).get(); diff --git a/java/com/google/gerrit/lucene/LuceneProjectIndex.java b/java/com/google/gerrit/lucene/LuceneProjectIndex.java index 7e7aecb923..11707be379 100644 --- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java +++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java @@ -125,6 +125,15 @@ public class LuceneProjectIndex extends AbstractLuceneIndex<Project.NameKey, Pro } @Override + public void insert(ProjectData projectState) { + try { + insert(toDocument(projectState)).get(); + } catch (ExecutionException | InterruptedException e) { + throw new StorageException(e); + } + } + + @Override public void delete(Project.NameKey nameKey) { try { delete(idTerm(nameKey)).get(); diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java index a12c08d7ff..b4b5ffbe55 100644 --- a/java/com/google/gerrit/pgm/Reindex.java +++ b/java/com/google/gerrit/pgm/Reindex.java @@ -17,9 +17,11 @@ package com.google.gerrit.pgm; import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toSet; +import com.google.common.cache.Cache; import com.google.common.collect.Sets; import com.google.gerrit.common.Die; import com.google.gerrit.extensions.config.FactoryModule; +import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.index.Index; import com.google.gerrit.index.IndexDefinition; import com.google.gerrit.index.IndexType; @@ -28,17 +30,24 @@ import com.google.gerrit.lifecycle.LifecycleManager; import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.pgm.util.BatchProgramModule; import com.google.gerrit.pgm.util.SiteProgram; +import com.google.gerrit.server.cache.CacheDisplay; +import com.google.gerrit.server.cache.CacheInfo; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.index.IndexModule; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; import com.google.gerrit.server.index.options.AutoFlush; +import com.google.gerrit.server.index.options.IsFirstInsertForEntry; import com.google.gerrit.server.plugins.PluginGuiceEnvironment; import com.google.gerrit.server.util.ReplicaUtil; +import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Key; import com.google.inject.Module; +import com.google.inject.multibindings.OptionalBinder; +import java.io.StringWriter; +import java.io.Writer; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; @@ -49,6 +58,8 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.util.io.NullOutputStream; import org.kohsuke.args4j.Option; @@ -79,6 +90,7 @@ public class Reindex extends SiteProgram { private Config globalConfig; @Inject private Collection<IndexDefinition<?, ?, ?>> indexDefs; + @Inject private DynamicMap<Cache<?, ?>> cacheMap; @Override public int run() throws Exception { @@ -176,6 +188,16 @@ public class Reindex extends SiteProgram { throw new IllegalStateException("unsupported index.type = " + indexType); } modules.add(indexModule); + modules.add( + new AbstractModule() { + @Override + protected void configure() { + super.configure(); + OptionalBinder.newOptionalBinder(binder(), IsFirstInsertForEntry.class) + .setBinding() + .toInstance(IsFirstInsertForEntry.YES); + } + }); modules.add(new BatchProgramModule(dbInjector)); modules.add( new FactoryModule() { @@ -223,6 +245,20 @@ public class Reindex extends SiteProgram { System.out.format( "Index %s in version %d is %sready\n", def.getName(), index.getSchema().getVersion(), result.success() ? "" : "NOT "); + + try (Writer sw = new StringWriter()) { + sw.write(String.format("Cache Statistics at the end of reindexing %s\n", def.getName())); + new CacheDisplay( + sw, + StreamSupport.stream(cacheMap.spliterator(), false) + .map(e -> new CacheInfo(e.getExportName(), e.get())) + .collect(Collectors.toList())) + .displayCaches(); + System.out.print(sw.toString()); + } catch (Exception e) { + System.out.format("Error displaying the cache statistics\n" + e.getMessage()); + } + return result.success(); } } diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java index b24fdad9f3..64eef050a6 100644 --- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java @@ -133,7 +133,7 @@ public class BatchProgramModule extends FactoryModule { // We're just running through each change // once, so don't worry about cache removal. bind(new TypeLiteral<DynamicSet<CacheRemovalListener>>() {}).toInstance(DynamicSet.emptySet()); - bind(new TypeLiteral<DynamicMap<Cache<?, ?>>>() {}).toInstance(DynamicMap.emptyMap()); + DynamicMap.mapOf(binder(), new TypeLiteral<Cache<?, ?>>() {}); bind(new TypeLiteral<List<CommentLinkInfo>>() {}) .toProvider(CommentLinkProvider.class) .in(SINGLETON); diff --git a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java index 6c76de7c62..60f3d4bbf0 100644 --- a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java +++ b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java @@ -121,7 +121,7 @@ public class CreateGroupPermissionSyncer implements ChangeMergedListener { }); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); } } diff --git a/java/com/google/gerrit/server/cache/CacheDisplay.java b/java/com/google/gerrit/server/cache/CacheDisplay.java new file mode 100644 index 0000000000..60f5186e8b --- /dev/null +++ b/java/com/google/gerrit/server/cache/CacheDisplay.java @@ -0,0 +1,129 @@ +// Copyright (C) 2021 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.cache; + +import com.google.common.base.Strings; +import java.io.IOException; +import java.io.Writer; +import java.util.Collection; + +public class CacheDisplay { + + private final Writer stdout; + private final int nw; + private final Collection<CacheInfo> caches; + + public CacheDisplay(Writer stdout, int nw, Collection<CacheInfo> caches) { + this.stdout = stdout; + this.nw = nw; + this.caches = caches; + } + + public CacheDisplay(Writer stdout, Collection<CacheInfo> caches) { + this(stdout, 30, caches); + } + + public void displayCaches() throws IOException { + stdout.write( + String.format( // + "%1s %-" + nw + "s|%-21s| %-5s |%-9s|\n" // + , + "" // + , + "Name" // + , + "Entries" // + , + "AvgGet" // + , + "Hit Ratio" // + )); + stdout.write( + String.format( // + "%1s %-" + nw + "s|%6s %6s %7s| %-5s |%-4s %-4s|\n" // + , + "" // + , + "" // + , + "Mem" // + , + "Disk" // + , + "Space" // + , + "" // + , + "Mem" // + , + "Disk" // + )); + stdout.write("--"); + for (int i = 0; i < nw; i++) { + stdout.write('-'); + } + stdout.write("+---------------------+---------+---------+\n"); + printMemoryCoreCaches(caches); + printMemoryPluginCaches(caches); + printDiskCaches(caches); + stdout.write('\n'); + } + + private void printMemoryCoreCaches(Collection<CacheInfo> caches) throws IOException { + for (CacheInfo cache : caches) { + if (!cache.name.contains("-") && CacheInfo.CacheType.MEM.equals(cache.type)) { + printCache(cache); + } + } + } + + private void printMemoryPluginCaches(Collection<CacheInfo> caches) throws IOException { + for (CacheInfo cache : caches) { + if (cache.name.contains("-") && CacheInfo.CacheType.MEM.equals(cache.type)) { + printCache(cache); + } + } + } + + private void printDiskCaches(Collection<CacheInfo> caches) throws IOException { + for (CacheInfo cache : caches) { + if (CacheInfo.CacheType.DISK.equals(cache.type)) { + printCache(cache); + } + } + } + + private void printCache(CacheInfo cache) throws IOException { + stdout.write( + String.format( + "%1s %-" + nw + "s|%6s %6s %7s| %7s |%4s %4s|\n", + CacheInfo.CacheType.DISK.equals(cache.type) ? "D" : "", + cache.name, + nullToEmpty(cache.entries.mem), + nullToEmpty(cache.entries.disk), + Strings.nullToEmpty(cache.entries.space), + Strings.nullToEmpty(cache.averageGet), + formatAsPercent(cache.hitRatio.mem), + formatAsPercent(cache.hitRatio.disk))); + } + + private static String nullToEmpty(Long l) { + return l != null ? String.valueOf(l) : ""; + } + + private static String formatAsPercent(Integer i) { + return i != null ? i + "%" : ""; + } +} diff --git a/java/com/google/gerrit/server/cache/CacheInfo.java b/java/com/google/gerrit/server/cache/CacheInfo.java new file mode 100644 index 0000000000..d6eb065339 --- /dev/null +++ b/java/com/google/gerrit/server/cache/CacheInfo.java @@ -0,0 +1,133 @@ +// Copyright (C) 2021 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.cache; + +import com.google.common.cache.Cache; +import com.google.common.cache.CacheStats; + +public class CacheInfo { + + public String name; + public CacheType type; + public EntriesInfo entries; + public String averageGet; + public HitRatioInfo hitRatio; + + public CacheInfo(Cache<?, ?> cache) { + this(null, cache); + } + + public CacheInfo(String name, Cache<?, ?> cache) { + this.name = name; + + CacheStats stat = cache.stats(); + + entries = new EntriesInfo(); + entries.setMem(cache.size()); + + averageGet = duration(stat.averageLoadPenalty()); + + hitRatio = new HitRatioInfo(); + hitRatio.setMem(stat.hitCount(), stat.requestCount()); + + if (cache instanceof PersistentCache) { + type = CacheType.DISK; + PersistentCache.DiskStats diskStats = ((PersistentCache) cache).diskStats(); + entries.setDisk(diskStats.size()); + entries.setSpace(diskStats.space()); + hitRatio.setDisk(diskStats.hitCount(), diskStats.requestCount()); + } else { + type = CacheType.MEM; + } + } + + private static String duration(double ns) { + if (ns < 0.5) { + return null; + } + String suffix = "ns"; + if (ns >= 1000.0) { + ns /= 1000.0; + suffix = "us"; + } + if (ns >= 1000.0) { + ns /= 1000.0; + suffix = "ms"; + } + if (ns >= 1000.0) { + ns /= 1000.0; + suffix = "s"; + } + return String.format("%4.1f%s", ns, suffix).trim(); + } + + public static class EntriesInfo { + public Long mem; + public Long disk; + public String space; + + public void setMem(long mem) { + this.mem = mem != 0 ? mem : null; + } + + public void setDisk(long disk) { + this.disk = disk != 0 ? disk : null; + } + + public void setSpace(double value) { + space = bytes(value); + } + + private static String bytes(double value) { + value /= 1024; + String suffix = "k"; + + if (value > 1024) { + value /= 1024; + suffix = "m"; + } + if (value > 1024) { + value /= 1024; + suffix = "g"; + } + return String.format("%1$6.2f%2$s", value, suffix).trim(); + } + } + + public static class HitRatioInfo { + public Integer mem; + public Integer disk; + + public void setMem(long value, long total) { + mem = percent(value, total); + } + + public void setDisk(long value, long total) { + disk = percent(value, total); + } + + private static Integer percent(long value, long total) { + if (total <= 0) { + return null; + } + return (int) ((100 * value) / total); + } + } + + public enum CacheType { + MEM, + DISK + } +} diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index f6c462c19e..55d7af00f6 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -3279,7 +3279,7 @@ class ReceiveCommits { } if (isConfig(cmd)) { logger.atFine().log("Reloading project in cache"); - projectCache.evict(project); + projectCache.evictAndReindex(project); ProjectState ps = projectCache.get(project.getNameKey()).orElseThrow(illegalState(project.getNameKey())); try { diff --git a/java/com/google/gerrit/server/group/db/RenameGroupOp.java b/java/com/google/gerrit/server/group/db/RenameGroupOp.java index 843b346684..257bc16d17 100644 --- a/java/com/google/gerrit/server/group/db/RenameGroupOp.java +++ b/java/com/google/gerrit/server/group/db/RenameGroupOp.java @@ -123,7 +123,7 @@ class RenameGroupOp extends DefaultQueueOp { // GroupReference ref = config.getGroup(uuid); if (ref == null || newName.equals(ref.getName())) { - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); return; } @@ -132,7 +132,7 @@ class RenameGroupOp extends DefaultQueueOp { md.setMessage("Rename group " + oldName + " to " + newName + "\n"); try { config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); success = true; } catch (IOException e) { logger.atSevere().withCause(e).log( diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java index 4050f43d9a..a7298638c1 100644 --- a/java/com/google/gerrit/server/index/IndexModule.java +++ b/java/com/google/gerrit/server/index/IndexModule.java @@ -52,6 +52,7 @@ import com.google.gerrit.server.index.group.GroupIndexRewriter; import com.google.gerrit.server.index.group.GroupIndexer; import com.google.gerrit.server.index.group.GroupIndexerImpl; import com.google.gerrit.server.index.group.GroupSchemaDefinitions; +import com.google.gerrit.server.index.options.IsFirstInsertForEntry; import com.google.gerrit.server.index.project.ProjectIndexDefinition; import com.google.gerrit.server.index.project.ProjectIndexerImpl; import com.google.inject.Inject; @@ -60,6 +61,7 @@ import com.google.inject.Key; import com.google.inject.Provides; import com.google.inject.ProvisionException; import com.google.inject.Singleton; +import com.google.inject.multibindings.OptionalBinder; import java.util.Collection; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -145,6 +147,9 @@ public class IndexModule extends LifecycleModule { } DynamicSet.setOf(binder(), OnlineUpgradeListener.class); + OptionalBinder.newOptionalBinder(binder(), IsFirstInsertForEntry.class) + .setDefault() + .toInstance(IsFirstInsertForEntry.NO); } @Provides diff --git a/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java b/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java index 63889b798d..1f48e35fd5 100644 --- a/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java +++ b/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java @@ -27,6 +27,7 @@ import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.Accounts; import com.google.gerrit.server.index.IndexExecutor; +import com.google.gerrit.server.index.options.IsFirstInsertForEntry; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; @@ -50,15 +51,18 @@ public class AllAccountsIndexer extends SiteIndexer<Account.Id, AccountState, Ac private final ListeningExecutorService executor; private final Accounts accounts; private final AccountCache accountCache; + private final IsFirstInsertForEntry isFirstInsertForEntry; @Inject AllAccountsIndexer( @IndexExecutor(BATCH) ListeningExecutorService executor, Accounts accounts, - AccountCache accountCache) { + AccountCache accountCache, + IsFirstInsertForEntry isFirstInsertForEntry) { this.executor = executor; this.accounts = accounts; this.accountCache = accountCache; + this.isFirstInsertForEntry = isFirstInsertForEntry; } @Override @@ -92,7 +96,11 @@ public class AllAccountsIndexer extends SiteIndexer<Account.Id, AccountState, Ac try { Optional<AccountState> a = accountCache.get(id); if (a.isPresent()) { - index.replace(a.get()); + if (isFirstInsertForEntry.equals(IsFirstInsertForEntry.YES)) { + index.insert(a.get()); + } else { + index.replace(a.get()); + } } else { index.delete(id); } diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java index 83b4e61cf2..bde614ffbb 100644 --- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java +++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java @@ -20,15 +20,16 @@ import static com.google.common.util.concurrent.Futures.transform; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH; +import com.google.auto.value.AutoValue; import com.google.common.base.Stopwatch; +import com.google.common.collect.Sets; import com.google.common.flogger.FluentLogger; -import com.google.common.primitives.Ints; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Project; -import com.google.gerrit.entities.RefNames; import com.google.gerrit.index.SiteIndexer; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MultiProgressMonitor; @@ -38,6 +39,7 @@ import com.google.gerrit.server.index.IndexExecutor; import com.google.gerrit.server.index.OnlineReindexMode; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult; +import com.google.gerrit.server.notedb.ChangeNotes.Factory.ScanResult; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.query.change.ChangeData; import com.google.inject.Inject; @@ -45,11 +47,13 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Objects; +import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.atomic.AtomicBoolean; -import org.eclipse.jgit.errors.RepositoryNotFoundException; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; @@ -64,6 +68,15 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change private static final int PROJECT_SLICE_MAX_REFS = 1000; private final MultiProgressMonitor.Factory multiProgressMonitorFactory; + + private static class ProjectsCollectionFailure extends Exception { + private static final long serialVersionUID = 1L; + + public ProjectsCollectionFailure(String message) { + super(message); + } + } + private final ChangeData.Factory changeDataFactory; private final GitRepositoryManager repoManager; private final ListeningExecutorService executor; @@ -89,103 +102,58 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change this.projectCache = projectCache; } - private static class ProjectSlice { - private final Project.NameKey name; - private final int slice; - private final int slices; + @AutoValue + public abstract static class ProjectSlice { + public abstract Project.NameKey name(); - ProjectSlice(Project.NameKey name, int slice, int slices) { - this.name = name; - this.slice = slice; - this.slices = slices; - } + public abstract int slice(); - public Project.NameKey getName() { - return name; - } + public abstract int slices(); - public int getSlice() { - return slice; - } + public abstract ScanResult scanResult(); - public int getSlices() { - return slices; + private static ProjectSlice create(Project.NameKey name, int slice, int slices, ScanResult sr) { + return new AutoValue_AllChangesIndexer_ProjectSlice(name, slice, slices, sr); } } @Override public Result indexAll(ChangeIndex index) { - ProgressMonitor pm = new TextProgressMonitor(); - pm.beginTask("Collecting projects", ProgressMonitor.UNKNOWN); - List<ProjectSlice> projectSlices = new ArrayList<>(); - int changeCount = 0; + // The simplest approach to distribute indexing would be to let each thread grab a project + // and index it fully. But if a site has one big project and 100s of small projects, then + // in the beginning all CPUs would be busy reindexing projects. But soon enough all small + // projects have been reindexed, and only the thread that reindexes the big project is + // still working. The other threads would idle. Reindexing the big project on a single + // thread becomes the critical path. Bringing in more CPUs would not speed up things. + // + // To avoid such situations, we split big repos into smaller parts and let + // the thread pool index these smaller parts. This splitting introduces an overhead in the + // workload setup and there might be additional slow-downs from multiple threads + // concurrently working on different parts of the same project. But for Wikimedia's Gerrit, + // which had 2 big projects, many middle sized ones, and lots of smaller ones, the + // splitting of repos into smaller parts reduced indexing time from 1.5 hours to 55 minutes + // in 2020. + Stopwatch sw = Stopwatch.createStarted(); - int projectsFailed = 0; - for (Project.NameKey name : projectCache.all()) { - try (Repository repo = repoManager.openRepository(name)) { - // The simplest approach to distribute indexing would be to let each thread grab a project - // and index it fully. But if a site has one big project and 100s of small projects, then - // in the beginning all CPUs would be busy reindexing projects. But soon enough all small - // projects have been reindexed, and only the thread that reindexes the big project is - // still working. The other threads would idle. Reindexing the big project on a single - // thread becomes the critical path. Bringing in more CPUs would not speed up things. - // - // To avoid such situations, we split big repos into smaller parts and let - // the thread pool index these smaller parts. This splitting introduces an overhead in the - // workload setup and there might be additional slow-downs from multiple threads - // concurrently working on different parts of the same project. But for Wikimedia's Gerrit, - // which had 2 big projects, many middle sized ones, and lots of smaller ones, the - // splitting of repos into smaller parts reduced indexing time from 1.5 hours to 55 minutes - // in 2020. - int size = estimateSize(repo); - if (size == 0) { - pm.update(1); - continue; - } - changeCount += size; - int slices = 1 + size / PROJECT_SLICE_MAX_REFS; - if (slices > 1) { - verboseWriter.println("Submitting " + name + " for indexing in " + slices + " slices"); - } - for (int slice = 0; slice < slices; slice++) { - projectSlices.add(new ProjectSlice(name, slice, slices)); - } - } catch (IOException e) { - logger.atSevere().withCause(e).log("Error collecting project %s", name); - projectsFailed++; - if (projectsFailed > projectCache.all().size() / 2) { - logger.atSevere().log("Over 50%% of the projects could not be collected: aborted"); - return Result.create(sw, false, 0, 0); - } - } - pm.update(1); + List<ProjectSlice> projectSlices; + try { + projectSlices = new SliceCreator().create(); + } catch (ProjectsCollectionFailure | InterruptedException | ExecutionException e) { + logger.atSevere().log(e.getMessage()); + return Result.create(sw, false, 0, 0); } - pm.endTask(); - setTotalWork(changeCount); - - // projectSlices are currently grouped by projects. First all slices for project1, followed - // by all slices for project2, and so on. As workers pick tasks sequentially, multiple threads - // would typically work concurrently on different slices of the same project. While this is not - // a big issue, shuffling the list beforehand helps with ungrouping the project slices, so - // different slices are less likely to be worked on concurrently. + + // Since project slices are created in parallel, they are somewhat shuffled already. However, + // the number of threads used to create the project slices doesn't guarantee good randomization. + // If the slices are not shuffled well, then multiple threads would typically work concurrently + // on different slices of the same project. While this is not a big issue, shuffling the list + // beforehand helps with ungrouping the project slices, so different slices are less likely to + // be worked on concurrently. // This shuffling gave a 6% runtime reduction for Wikimedia's Gerrit in 2020. Collections.shuffle(projectSlices); return indexAll(index, projectSlices); } - private int estimateSize(Repository repo) throws IOException { - // Estimate size based on IDs that show up in ref names. This is not perfect, since patch set - // refs may exist for changes whose metadata was never successfully stored. But that's ok, as - // the estimate is just used as a heuristic for sorting projects. - long size = - repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES).stream() - .map(r -> Change.Id.fromRef(r.getName())) - .filter(Objects::nonNull) - .distinct() - .count(); - return Ints.saturatedCast(size); - } - private SiteIndexer.Result indexAll(ChangeIndex index, List<ProjectSlice> projectSlices) { Stopwatch sw = Stopwatch.createStarted(); MultiProgressMonitor mpm = @@ -199,9 +167,9 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change AtomicBoolean ok = new AtomicBoolean(true); for (ProjectSlice projectSlice : projectSlices) { - Project.NameKey name = projectSlice.getName(); - int slice = projectSlice.getSlice(); - int slices = projectSlice.getSlices(); + Project.NameKey name = projectSlice.name(); + int slice = projectSlice.slice(); + int slices = projectSlice.slices(); ListenableFuture<?> future = executor.submit( reindexProject( @@ -209,6 +177,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change name, slice, slices, + projectSlice.scanResult(), doneTask, failedTask)); String description = "project " + name + " (" + slice + "/" + slices + ")"; @@ -249,7 +218,13 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change public Callable<Void> reindexProject( ChangeIndexer indexer, Project.NameKey project, Task done, Task failed) { - return reindexProject(indexer, project, 0, 1, done, failed); + try (Repository repo = repoManager.openRepository(project)) { + return reindexProject( + indexer, project, 0, 1, ChangeNotes.Factory.scanChangeIds(repo), done, failed); + } catch (IOException e) { + logger.atSevere().log(e.getMessage()); + return null; + } } public Callable<Void> reindexProject( @@ -257,9 +232,10 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change Project.NameKey project, int slice, int slices, + ScanResult scanResult, Task done, Task failed) { - return new ProjectIndexer(indexer, project, slice, slices, done, failed); + return new ProjectIndexer(indexer, project, slice, slices, scanResult, done, failed); } private class ProjectIndexer implements Callable<Void> { @@ -267,6 +243,7 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change private final Project.NameKey project; private final int slice; private final int slices; + private final ScanResult scanResult; private final ProgressMonitor done; private final ProgressMonitor failed; @@ -275,32 +252,30 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change Project.NameKey project, int slice, int slices, + ScanResult scanResult, ProgressMonitor done, ProgressMonitor failed) { this.indexer = indexer; this.project = project; this.slice = slice; this.slices = slices; + this.scanResult = scanResult; this.done = done; this.failed = failed; } @Override public Void call() throws Exception { - try (Repository repo = repoManager.openRepository(project)) { - OnlineReindexMode.begin(); - - // Order of scanning changes is undefined. This is ok if we assume that packfile locality is - // not important for indexing, since sites should have a fully populated DiffSummary cache. - // It does mean that reindexing after invalidating the DiffSummary cache will be expensive, - // but the goal is to invalidate that cache as infrequently as we possibly can. And besides, - // we don't have concrete proof that improving packfile locality would help. - notesFactory.scan(repo, project, id -> (id.get() % slices) == slice).forEach(r -> index(r)); - } catch (RepositoryNotFoundException rnfe) { - logger.atSevere().log(rnfe.getMessage()); - } finally { - OnlineReindexMode.end(); - } + OnlineReindexMode.begin(); + // Order of scanning changes is undefined. This is ok if we assume that packfile locality is + // not important for indexing, since sites should have a fully populated DiffSummary cache. + // It does mean that reindexing after invalidating the DiffSummary cache will be expensive, + // but the goal is to invalidate that cache as infrequently as we possibly can. And besides, + // we don't have concrete proof that improving packfile locality would help. + notesFactory + .scan(scanResult, project, id -> (id.get() % slices) == slice) + .forEach(r -> index(r)); + OnlineReindexMode.end(); return null; } @@ -340,4 +315,63 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change return "Index all changes of project " + project.get(); } } + + private class SliceCreator { + final Set<ProjectSlice> projectSlices = Sets.newConcurrentHashSet(); + final AtomicInteger changeCount = new AtomicInteger(0); + final AtomicInteger projectsFailed = new AtomicInteger(0); + final ProgressMonitor pm = new TextProgressMonitor(); + + private List<ProjectSlice> create() + throws ProjectsCollectionFailure, InterruptedException, ExecutionException { + List<ListenableFuture<?>> futures = new ArrayList<>(); + pm.beginTask("Collecting projects", ProgressMonitor.UNKNOWN); + for (Project.NameKey name : projectCache.all()) { + futures.add(executor.submit(new ProjectSliceCreator(name))); + } + + Futures.allAsList(futures).get(); + + if (projectsFailed.get() > projectCache.all().size() / 2) { + throw new ProjectsCollectionFailure( + "Over 50%% of the projects could not be collected: aborted"); + } + + pm.endTask(); + setTotalWork(changeCount.get()); + return projectSlices.stream().collect(Collectors.toList()); + } + + private class ProjectSliceCreator implements Callable<Void> { + final Project.NameKey name; + + public ProjectSliceCreator(Project.NameKey name) { + this.name = name; + } + + @Override + public Void call() throws IOException { + try (Repository repo = repoManager.openRepository(name)) { + ScanResult sr = ChangeNotes.Factory.scanChangeIds(repo); + int size = sr.all().size(); + if (size > 0) { + changeCount.addAndGet(size); + int slices = 1 + size / PROJECT_SLICE_MAX_REFS; + if (slices > 1) { + verboseWriter.println( + "Submitting " + name + " for indexing in " + slices + " slices"); + } + for (int slice = 0; slice < slices; slice++) { + projectSlices.add(ProjectSlice.create(name, slice, slices, sr)); + } + } + } catch (IOException e) { + logger.atSevere().withCause(e).log("Error collecting project %s", name); + projectsFailed.incrementAndGet(); + } + pm.update(1); + return null; + } + } + } } diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java index f7f0f33166..8f68904164 100644 --- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java +++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java @@ -30,6 +30,7 @@ import com.google.gerrit.index.Index; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.index.IndexExecutor; import com.google.gerrit.server.index.StalenessCheckResult; +import com.google.gerrit.server.index.options.IsFirstInsertForEntry; import com.google.gerrit.server.logging.Metadata; import com.google.gerrit.server.logging.TraceContext; import com.google.gerrit.server.logging.TraceContext.TraceTimer; @@ -79,6 +80,7 @@ public class ChangeIndexer { private final PluginSetContext<ChangeIndexedListener> indexedListeners; private final StalenessChecker stalenessChecker; private final boolean autoReindexIfStale; + private final IsFirstInsertForEntry isFirstInsertForEntry; private final Map<Change.Id, IndexTask> queuedIndexTasks = new ConcurrentHashMap<>(); private final Set<ReindexIfStaleTask> queuedReindexIfStaleTasks = @@ -94,7 +96,8 @@ public class ChangeIndexer { StalenessChecker stalenessChecker, @IndexExecutor(BATCH) ListeningExecutorService batchExecutor, @Assisted ListeningExecutorService executor, - @Assisted ChangeIndex index) { + @Assisted ChangeIndex index, + IsFirstInsertForEntry isFirstInsertForEntry) { this.executor = executor; this.changeDataFactory = changeDataFactory; this.notesFactory = notesFactory; @@ -105,6 +108,7 @@ public class ChangeIndexer { this.autoReindexIfStale = autoReindexIfStale(cfg); this.index = index; this.indexes = null; + this.isFirstInsertForEntry = isFirstInsertForEntry; } @AssistedInject @@ -117,7 +121,8 @@ public class ChangeIndexer { StalenessChecker stalenessChecker, @IndexExecutor(BATCH) ListeningExecutorService batchExecutor, @Assisted ListeningExecutorService executor, - @Assisted ChangeIndexCollection indexes) { + @Assisted ChangeIndexCollection indexes, + IsFirstInsertForEntry isFirstInsertForEntry) { this.executor = executor; this.changeDataFactory = changeDataFactory; this.notesFactory = notesFactory; @@ -128,6 +133,7 @@ public class ChangeIndexer { this.autoReindexIfStale = autoReindexIfStale(cfg); this.index = null; this.indexes = indexes; + this.isFirstInsertForEntry = isFirstInsertForEntry; } private static boolean autoReindexIfStale(Config cfg) { @@ -227,21 +233,25 @@ public class ChangeIndexer { } private void indexImpl(ChangeData cd) { - logger.atFine().log("Replace change %d in index.", cd.getId().get()); + logger.atFine().log("Reindex change %d in index.", cd.getId().get()); for (Index<?, ChangeData> i : getWriteIndexes()) { try (TraceTimer traceTimer = TraceContext.newTimer( - "Replacing change in index", + "Reindexing change in index", Metadata.builder() .changeId(cd.getId().get()) .patchSetId(cd.currentPatchSet().number()) .indexVersion(i.getSchema().getVersion()) .build())) { - i.replace(cd); + if (isFirstInsertForEntry.equals(IsFirstInsertForEntry.YES)) { + i.insert(cd); + } else { + i.replace(cd); + } } catch (RuntimeException e) { throw new StorageException( String.format( - "Failed to replace change %d in index version %d (current patch set = %d)", + "Failed to reindex change %d in index version %d (current patch set = %d)", cd.getId().get(), i.getSchema().getVersion(), cd.currentPatchSet().number()), e); } diff --git a/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java b/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java index b3ef679675..3773d4354d 100644 --- a/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java +++ b/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java @@ -30,6 +30,7 @@ import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.group.db.Groups; import com.google.gerrit.server.group.db.GroupsNoteDbConsistencyChecker; import com.google.gerrit.server.index.IndexExecutor; +import com.google.gerrit.server.index.options.IsFirstInsertForEntry; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; @@ -54,15 +55,18 @@ public class AllGroupsIndexer extends SiteIndexer<AccountGroup.UUID, InternalGro private final ListeningExecutorService executor; private final GroupCache groupCache; private final Groups groups; + private final IsFirstInsertForEntry isFirstInsertForEntry; @Inject AllGroupsIndexer( @IndexExecutor(BATCH) ListeningExecutorService executor, GroupCache groupCache, - Groups groups) { + Groups groups, + IsFirstInsertForEntry isFirstInsertForEntry) { this.executor = executor; this.groupCache = groupCache; this.groups = groups; + this.isFirstInsertForEntry = isFirstInsertForEntry; } @Override @@ -96,9 +100,14 @@ public class AllGroupsIndexer extends SiteIndexer<AccountGroup.UUID, InternalGro executor.submit( () -> { try { + groupCache.evict(uuid); InternalGroup internalGroup = reindexedGroups.get(uuid); if (internalGroup != null) { - index.replace(internalGroup); + if (isFirstInsertForEntry.equals(IsFirstInsertForEntry.YES)) { + index.insert(internalGroup); + } else { + index.replace(internalGroup); + } } else { index.delete(uuid); diff --git a/java/com/google/gerrit/server/index/options/IsFirstInsertForEntry.java b/java/com/google/gerrit/server/index/options/IsFirstInsertForEntry.java new file mode 100644 index 0000000000..f943309e07 --- /dev/null +++ b/java/com/google/gerrit/server/index/options/IsFirstInsertForEntry.java @@ -0,0 +1,26 @@ +// Copyright (C) 2021 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.index.options; + +/** + * This enum should be injected and checked to decide on which operation ({@link + * com.google.gerrit.index.Index#replace(Object) replace()} or {@link + * com.google.gerrit.index.Index#insert(Object) insert()}) should be performed on a specific {@link + * com.google.gerrit.index.Index index}. + */ +public enum IsFirstInsertForEntry { + YES, + NO +} diff --git a/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java b/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java index 0e4b688d6c..1c977d168c 100644 --- a/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java +++ b/java/com/google/gerrit/server/index/project/AllProjectsIndexer.java @@ -27,6 +27,7 @@ import com.google.gerrit.index.SiteIndexer; import com.google.gerrit.index.project.ProjectData; import com.google.gerrit.index.project.ProjectIndex; import com.google.gerrit.server.index.IndexExecutor; +import com.google.gerrit.server.index.options.IsFirstInsertForEntry; import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -48,12 +49,16 @@ public class AllProjectsIndexer extends SiteIndexer<Project.NameKey, ProjectData private final ListeningExecutorService executor; private final ProjectCache projectCache; + private final IsFirstInsertForEntry isFirstInsertForEntry; @Inject AllProjectsIndexer( - @IndexExecutor(BATCH) ListeningExecutorService executor, ProjectCache projectCache) { + @IndexExecutor(BATCH) ListeningExecutorService executor, + ProjectCache projectCache, + IsFirstInsertForEntry isFirstInsertForEntry) { this.executor = executor; this.projectCache = projectCache; + this.isFirstInsertForEntry = isFirstInsertForEntry; } @Override @@ -79,8 +84,13 @@ public class AllProjectsIndexer extends SiteIndexer<Project.NameKey, ProjectData () -> { try { projectCache.evict(name); - index.replace( - projectCache.get(name).orElseThrow(illegalState(name)).toProjectData()); + ProjectData projectData = + projectCache.get(name).orElseThrow(illegalState(name)).toProjectData(); + if (isFirstInsertForEntry.equals(IsFirstInsertForEntry.YES)) { + index.insert(projectData); + } else { + index.replace(projectData); + } verboseWriter.println("Reindexed " + desc); done.incrementAndGet(); } catch (Exception e) { diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java index 5db3764d97..a1d6b29d14 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -115,6 +115,29 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> { this.projectCache = projectCache; } + @AutoValue + public abstract static class ScanResult { + abstract ImmutableSet<Change.Id> fromPatchSetRefs(); + + abstract ImmutableSet<Change.Id> fromMetaRefs(); + + public SetView<Change.Id> all() { + return Sets.union(fromPatchSetRefs(), fromMetaRefs()); + } + } + + public static ScanResult scanChangeIds(Repository repo) throws IOException { + ImmutableSet.Builder<Change.Id> fromPs = ImmutableSet.builder(); + ImmutableSet.Builder<Change.Id> fromMeta = ImmutableSet.builder(); + for (Ref r : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) { + Change.Id id = Change.Id.fromRef(r.getName()); + if (id != null) { + (r.getName().endsWith(RefNames.META_SUFFIX) ? fromMeta : fromPs).add(id); + } + } + return new AutoValue_ChangeNotes_Factory_ScanResult(fromPs.build(), fromMeta.build()); + } + public ChangeNotes createChecked(Change c) { return createChecked(c.getProject(), c.getId()); } @@ -274,8 +297,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> { public Stream<ChangeNotesResult> scan( Repository repo, Project.NameKey project, Predicate<Change.Id> changeIdPredicate) throws IOException { - ScanResult sr = scanChangeIds(repo); + return scan(scanChangeIds(repo), project, changeIdPredicate); + } + public Stream<ChangeNotesResult> scan( + ScanResult sr, Project.NameKey project, Predicate<Change.Id> changeIdPredicate) { Stream<Change.Id> idStream = sr.all().stream(); if (changeIdPredicate != null) { idStream = idStream.filter(changeIdPredicate); @@ -347,29 +373,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> { @Nullable abstract ChangeNotes maybeNotes(); } - - @AutoValue - abstract static class ScanResult { - abstract ImmutableSet<Change.Id> fromPatchSetRefs(); - - abstract ImmutableSet<Change.Id> fromMetaRefs(); - - SetView<Change.Id> all() { - return Sets.union(fromPatchSetRefs(), fromMetaRefs()); - } - } - - private static ScanResult scanChangeIds(Repository repo) throws IOException { - ImmutableSet.Builder<Change.Id> fromPs = ImmutableSet.builder(); - ImmutableSet.Builder<Change.Id> fromMeta = ImmutableSet.builder(); - for (Ref r : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) { - Change.Id id = Change.Id.fromRef(r.getName()); - if (id != null) { - (r.getName().endsWith(RefNames.META_SUFFIX) ? fromMeta : fromPs).add(id); - } - } - return new AutoValue_ChangeNotes_Factory_ScanResult(fromPs.build(), fromMeta.build()); - } } private final boolean shouldExist; diff --git a/java/com/google/gerrit/server/project/NullProjectCache.java b/java/com/google/gerrit/server/project/NullProjectCache.java index 1d5f5b7ee7..d19a726673 100644 --- a/java/com/google/gerrit/server/project/NullProjectCache.java +++ b/java/com/google/gerrit/server/project/NullProjectCache.java @@ -42,11 +42,6 @@ public class NullProjectCache implements ProjectCache { } @Override - public void evict(Project p) { - throw new UnsupportedOperationException(); - } - - @Override public void evict(NameKey p) { throw new UnsupportedOperationException(); } @@ -80,4 +75,14 @@ public class NullProjectCache implements ProjectCache { public void onCreateProject(NameKey newProjectName) throws IOException { throw new UnsupportedOperationException(); } + + @Override + public void evictAndReindex(Project p) { + throw new UnsupportedOperationException(); + } + + @Override + public void evictAndReindex(NameKey p) { + throw new UnsupportedOperationException(); + } } diff --git a/java/com/google/gerrit/server/project/ProjectCache.java b/java/com/google/gerrit/server/project/ProjectCache.java index fee7105a69..fb0a4ec2f2 100644 --- a/java/com/google/gerrit/server/project/ProjectCache.java +++ b/java/com/google/gerrit/server/project/ProjectCache.java @@ -59,18 +59,25 @@ public interface ProjectCache { Optional<ProjectState> get(@Nullable Project.NameKey projectName) throws StorageException; /** + * Invalidate the cached information about the given project. + * + * @param p the NameKey of the project that is being evicted + */ + void evict(Project.NameKey p); + + /** * Invalidate the cached information about the given project, and triggers reindexing for it * * @param p project that is being evicted */ - void evict(Project p); + void evictAndReindex(Project p); /** * Invalidate the cached information about the given project, and triggers reindexing for it * * @param p the NameKey of the project that is being evicted */ - void evict(Project.NameKey p); + void evictAndReindex(Project.NameKey p); /** * Remove information about the given project from the cache. It will no longer be returned from diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java index de27afa8f8..7e7c7bea24 100644 --- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java +++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java @@ -214,16 +214,21 @@ public class ProjectCacheImpl implements ProjectCache { } @Override - public void evict(Project p) { - evict(p.getNameKey()); - } - - @Override public void evict(Project.NameKey p) { if (p != null) { logger.atFine().log("Evict project '%s'", p.get()); inMemoryProjectCache.invalidate(p); } + } + + @Override + public void evictAndReindex(Project p) { + evictAndReindex(p.getNameKey()); + } + + @Override + public void evictAndReindex(Project.NameKey p) { + evict(p); indexer.get().index(p); } @@ -244,7 +249,7 @@ public class ProjectCacheImpl implements ProjectCache { } finally { listLock.unlock(); } - evict(name); + evictAndReindex(name); } @Override diff --git a/java/com/google/gerrit/server/restapi/config/GetCache.java b/java/com/google/gerrit/server/restapi/config/GetCache.java index 93600ea582..5dd3d3dc48 100644 --- a/java/com/google/gerrit/server/restapi/config/GetCache.java +++ b/java/com/google/gerrit/server/restapi/config/GetCache.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.restapi.config; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.server.cache.CacheInfo; import com.google.gerrit.server.config.CacheResource; import com.google.inject.Singleton; @@ -23,7 +24,7 @@ import com.google.inject.Singleton; public class GetCache implements RestReadView<CacheResource> { @Override - public Response<ListCaches.CacheInfo> apply(CacheResource rsrc) { - return Response.ok(new ListCaches.CacheInfo(rsrc.getName(), rsrc.getCache())); + public Response<CacheInfo> apply(CacheResource rsrc) { + return Response.ok(new CacheInfo(rsrc.getName(), rsrc.getCache())); } } diff --git a/java/com/google/gerrit/server/restapi/config/ListCaches.java b/java/com/google/gerrit/server/restapi/config/ListCaches.java index ccafbe89b1..ffc65c92da 100644 --- a/java/com/google/gerrit/server/restapi/config/ListCaches.java +++ b/java/com/google/gerrit/server/restapi/config/ListCaches.java @@ -22,7 +22,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.joining; import com.google.common.cache.Cache; -import com.google.common.cache.CacheStats; import com.google.common.collect.Streams; import com.google.gerrit.extensions.annotations.RequiresAnyCapability; import com.google.gerrit.extensions.registration.DynamicMap; @@ -30,7 +29,7 @@ import com.google.gerrit.extensions.registration.Extension; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.cache.PersistentCache; +import com.google.gerrit.server.cache.CacheInfo; import com.google.gerrit.server.config.ConfigResource; import com.google.inject.Inject; import java.util.Map; @@ -87,118 +86,4 @@ public class ListCaches implements RestReadView<ConfigResource> { } return Response.ok(cacheNames.collect(toImmutableList())); } - - public enum CacheType { - MEM, - DISK - } - - public static class CacheInfo { - public String name; - public CacheType type; - public EntriesInfo entries; - public String averageGet; - public HitRatioInfo hitRatio; - - public CacheInfo(Cache<?, ?> cache) { - this(null, cache); - } - - public CacheInfo(String name, Cache<?, ?> cache) { - this.name = name; - - CacheStats stat = cache.stats(); - - entries = new EntriesInfo(); - entries.setMem(cache.size()); - - averageGet = duration(stat.averageLoadPenalty()); - - hitRatio = new HitRatioInfo(); - hitRatio.setMem(stat.hitCount(), stat.requestCount()); - - if (cache instanceof PersistentCache) { - type = CacheType.DISK; - PersistentCache.DiskStats diskStats = ((PersistentCache) cache).diskStats(); - entries.setDisk(diskStats.size()); - entries.setSpace(diskStats.space()); - hitRatio.setDisk(diskStats.hitCount(), diskStats.requestCount()); - } else { - type = CacheType.MEM; - } - } - - private static String duration(double ns) { - if (ns < 0.5) { - return null; - } - String suffix = "ns"; - if (ns >= 1000.0) { - ns /= 1000.0; - suffix = "us"; - } - if (ns >= 1000.0) { - ns /= 1000.0; - suffix = "ms"; - } - if (ns >= 1000.0) { - ns /= 1000.0; - suffix = "s"; - } - return String.format("%4.1f%s", ns, suffix).trim(); - } - } - - public static class EntriesInfo { - public Long mem; - public Long disk; - public String space; - - public void setMem(long mem) { - this.mem = mem != 0 ? mem : null; - } - - public void setDisk(long disk) { - this.disk = disk != 0 ? disk : null; - } - - public void setSpace(double value) { - space = bytes(value); - } - - private static String bytes(double value) { - value /= 1024; - String suffix = "k"; - - if (value > 1024) { - value /= 1024; - suffix = "m"; - } - if (value > 1024) { - value /= 1024; - suffix = "g"; - } - return String.format("%1$6.2f%2$s", value, suffix).trim(); - } - } - - public static class HitRatioInfo { - public Integer mem; - public Integer disk; - - public void setMem(long value, long total) { - mem = percent(value, total); - } - - public void setDisk(long value, long total) { - disk = percent(value, total); - } - - private static Integer percent(long value, long total) { - if (total <= 0) { - return null; - } - return (int) ((100 * value) / total); - } - } } diff --git a/java/com/google/gerrit/server/restapi/project/CreateLabel.java b/java/com/google/gerrit/server/restapi/project/CreateLabel.java index d2f41613bb..e3f293a36a 100644 --- a/java/com/google/gerrit/server/restapi/project/CreateLabel.java +++ b/java/com/google/gerrit/server/restapi/project/CreateLabel.java @@ -106,7 +106,7 @@ public class CreateLabel config.commit(md); - projectCache.evict(rsrc.getProjectState().getProject()); + projectCache.evictAndReindex(rsrc.getProjectState().getProject()); return Response.created(LabelDefinitionJson.format(rsrc.getNameKey(), labelType)); } diff --git a/java/com/google/gerrit/server/restapi/project/DeleteLabel.java b/java/com/google/gerrit/server/restapi/project/DeleteLabel.java index 531640cf38..8a1927aea2 100644 --- a/java/com/google/gerrit/server/restapi/project/DeleteLabel.java +++ b/java/com/google/gerrit/server/restapi/project/DeleteLabel.java @@ -90,7 +90,7 @@ public class DeleteLabel implements RestModifyView<LabelResource, InputWithCommi config.commit(md); } - projectCache.evict(rsrc.getProject().getProjectState().getProject()); + projectCache.evictAndReindex(rsrc.getProject().getProjectState().getProject()); return Response.none(); } diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java index a79439c1b4..651e7f078f 100644 --- a/java/com/google/gerrit/server/restapi/project/GetAccess.java +++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java @@ -153,12 +153,12 @@ public class GetAccess implements RestReadView<ProjectResource> { if (config.updateGroupNames(groupBackend)) { md.setMessage("Update group names\n"); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); projectState = projectCache.get(projectName).orElseThrow(illegalState(projectName)); perm = permissionBackend.currentUser().project(projectName); } else if (config.getRevision() != null && !config.getRevision().equals(projectState.getConfig().getRevision().orElse(null))) { - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); projectState = projectCache.get(projectName).orElseThrow(illegalState(projectName)); perm = permissionBackend.currentUser().project(projectName); } diff --git a/java/com/google/gerrit/server/restapi/project/PostLabels.java b/java/com/google/gerrit/server/restapi/project/PostLabels.java index a56cfe6c02..6cb912ebb7 100644 --- a/java/com/google/gerrit/server/restapi/project/PostLabels.java +++ b/java/com/google/gerrit/server/restapi/project/PostLabels.java @@ -139,7 +139,7 @@ public class PostLabels if (dirty) { config.commit(md); - projectCache.evict(rsrc.getProjectState().getProject()); + projectCache.evictAndReindex(rsrc.getProjectState().getProject()); } } diff --git a/java/com/google/gerrit/server/restapi/project/PutConfig.java b/java/com/google/gerrit/server/restapi/project/PutConfig.java index fa8638e734..30d667c468 100644 --- a/java/com/google/gerrit/server/restapi/project/PutConfig.java +++ b/java/com/google/gerrit/server/restapi/project/PutConfig.java @@ -163,7 +163,7 @@ public class PutConfig implements RestModifyView<ProjectResource, ConfigInput> { md.setMessage("Modified project settings\n"); try { projectConfig.commit(md); - projectCache.evict(projectConfig.getProject()); + projectCache.evictAndReindex(projectConfig.getProject()); md.getRepository().setGitwebDescription(projectConfig.getProject().getDescription()); } catch (IOException e) { if (e.getCause() instanceof ConfigInvalidException) { diff --git a/java/com/google/gerrit/server/restapi/project/PutDescription.java b/java/com/google/gerrit/server/restapi/project/PutDescription.java index a65c6260e2..ec42035ffc 100644 --- a/java/com/google/gerrit/server/restapi/project/PutDescription.java +++ b/java/com/google/gerrit/server/restapi/project/PutDescription.java @@ -84,7 +84,7 @@ public class PutDescription implements RestModifyView<ProjectResource, Descripti md.setAuthor(user); md.setMessage(msg); config.commit(md); - cache.evict(resource.getProjectState().getProject()); + cache.evictAndReindex(resource.getProjectState().getProject()); md.getRepository().setGitwebDescription(config.getProject().getDescription()); return Strings.isNullOrEmpty(config.getProject().getDescription()) diff --git a/java/com/google/gerrit/server/restapi/project/SetAccess.java b/java/com/google/gerrit/server/restapi/project/SetAccess.java index 794cae840c..07dbeca490 100644 --- a/java/com/google/gerrit/server/restapi/project/SetAccess.java +++ b/java/com/google/gerrit/server/restapi/project/SetAccess.java @@ -125,7 +125,7 @@ public class SetAccess implements RestModifyView<ProjectResource, ProjectAccessI } config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); createGroupPermissionSyncer.syncIfNeeded(); } catch (InvalidNameException e) { throw new BadRequestException(e.toString()); diff --git a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java index 5aef76a30c..853d7dff0e 100644 --- a/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java +++ b/java/com/google/gerrit/server/restapi/project/SetDefaultDashboard.java @@ -115,7 +115,7 @@ class SetDefaultDashboard implements RestModifyView<DashboardResource, SetDashbo md.setAuthor(rsrc.getUser().asIdentifiedUser()); md.setMessage(msg); config.commit(md); - cache.evict(rsrc.getProjectState().getProject()); + cache.evictAndReindex(rsrc.getProjectState().getProject()); if (target != null) { Response<DashboardInfo> response = get.get().apply(target); diff --git a/java/com/google/gerrit/server/restapi/project/SetLabel.java b/java/com/google/gerrit/server/restapi/project/SetLabel.java index d69abefae6..801fac7992 100644 --- a/java/com/google/gerrit/server/restapi/project/SetLabel.java +++ b/java/com/google/gerrit/server/restapi/project/SetLabel.java @@ -98,7 +98,7 @@ public class SetLabel implements RestModifyView<LabelResource, LabelDefinitionIn config.getLabelSections().get(newName.isEmpty() ? labelType.getName() : newName); config.commit(md); - projectCache.evict(rsrc.getProject().getProjectState().getProject()); + projectCache.evictAndReindex(rsrc.getProject().getProjectState().getProject()); } } return Response.ok(LabelDefinitionJson.format(rsrc.getProject().getNameKey(), labelType)); diff --git a/java/com/google/gerrit/server/restapi/project/SetParent.java b/java/com/google/gerrit/server/restapi/project/SetParent.java index 91c29f5332..ef31dc5b0b 100644 --- a/java/com/google/gerrit/server/restapi/project/SetParent.java +++ b/java/com/google/gerrit/server/restapi/project/SetParent.java @@ -114,7 +114,7 @@ public class SetParent md.setAuthor(user); md.setMessage(msg); config.commit(md); - cache.evict(rsrc.getProjectState().getProject()); + cache.evictAndReindex(rsrc.getProjectState().getProject()); Project.NameKey parent = config.getProject().getParent(allProjects); requireNonNull(parent); diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index 7d428eb701..f26ec17ec4 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -482,7 +482,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { // TODO(dborowitz): Move to BatchUpdate? Would also allow us to run once // per project even if multiple changes to refs/meta/config are submitted. if (RefNames.REFS_CONFIG.equals(getDest().branch())) { - args.projectCache.evict(getProject()); + args.projectCache.evictAndReindex(getProject()); ProjectState p = args.projectCache.get(getProject()).orElseThrow(illegalState(getProject())); try (Repository git = args.repoManager.openRepository(getProject())) { diff --git a/java/com/google/gerrit/sshd/commands/ShowCaches.java b/java/com/google/gerrit/sshd/commands/ShowCaches.java index 979be1be9c..02956f76a5 100644 --- a/java/com/google/gerrit/sshd/commands/ShowCaches.java +++ b/java/com/google/gerrit/sshd/commands/ShowCaches.java @@ -24,6 +24,8 @@ import com.google.gerrit.extensions.annotations.RequiresAnyCapability; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.cache.CacheDisplay; +import com.google.gerrit.server.cache.CacheInfo; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; @@ -35,8 +37,6 @@ import com.google.gerrit.server.restapi.config.GetSummary.SummaryInfo; import com.google.gerrit.server.restapi.config.GetSummary.TaskSummaryInfo; import com.google.gerrit.server.restapi.config.GetSummary.ThreadSummaryInfo; import com.google.gerrit.server.restapi.config.ListCaches; -import com.google.gerrit.server.restapi.config.ListCaches.CacheInfo; -import com.google.gerrit.server.restapi.config.ListCaches.CacheType; import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; @@ -123,52 +123,8 @@ final class ShowCaches extends SshCommand { stdout.format("%-25s %-20s uptime %16s\n", "", "", uptime(now.getTime() - serverStarted)); stdout.print('\n'); - stdout.print( - String.format( // - "%1s %-" + nw + "s|%-21s| %-5s |%-9s|\n" // - , - "" // - , - "Name" // - , - "Entries" // - , - "AvgGet" // - , - "Hit Ratio" // - )); - stdout.print( - String.format( // - "%1s %-" + nw + "s|%6s %6s %7s| %-5s |%-4s %-4s|\n" // - , - "" // - , - "" // - , - "Mem" // - , - "Disk" // - , - "Space" // - , - "" // - , - "Mem" // - , - "Disk" // - )); - stdout.print("--"); - for (int i = 0; i < nw; i++) { - stdout.print('-'); - } - stdout.print("+---------------------+---------+---------+\n"); - try { - Collection<CacheInfo> caches = getCaches(); - printMemoryCoreCaches(caches); - printMemoryPluginCaches(caches); - printDiskCaches(caches); - stdout.print('\n'); + new CacheDisplay(stdout, nw, getCaches()).displayCaches(); boolean showJvm; try { @@ -209,52 +165,6 @@ final class ShowCaches extends SshCommand { return caches.values(); } - private void printMemoryCoreCaches(Collection<CacheInfo> caches) { - for (CacheInfo cache : caches) { - if (!cache.name.contains("-") && CacheType.MEM.equals(cache.type)) { - printCache(cache); - } - } - } - - private void printMemoryPluginCaches(Collection<CacheInfo> caches) { - for (CacheInfo cache : caches) { - if (cache.name.contains("-") && CacheType.MEM.equals(cache.type)) { - printCache(cache); - } - } - } - - private void printDiskCaches(Collection<CacheInfo> caches) { - for (CacheInfo cache : caches) { - if (CacheType.DISK.equals(cache.type)) { - printCache(cache); - } - } - } - - private void printCache(CacheInfo cache) { - stdout.print( - String.format( - "%1s %-" + nw + "s|%6s %6s %7s| %7s |%4s %4s|\n", - CacheType.DISK.equals(cache.type) ? "D" : "", - cache.name, - nullToEmpty(cache.entries.mem), - nullToEmpty(cache.entries.disk), - Strings.nullToEmpty(cache.entries.space), - Strings.nullToEmpty(cache.averageGet), - formatAsPercent(cache.hitRatio.mem), - formatAsPercent(cache.hitRatio.disk))); - } - - private static String nullToEmpty(Long l) { - return l != null ? String.valueOf(l) : ""; - } - - private static String formatAsPercent(Integer i) { - return i != null ? String.valueOf(i) + "%" : ""; - } - private void memSummary(MemSummaryInfo memSummary) { stdout.format( "Mem: %s total = %s used + %s free + %s buffers\n", diff --git a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java index ff9bac9afb..7d04558561 100644 --- a/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java +++ b/javatests/com/google/gerrit/acceptance/ProjectResetterTest.java @@ -231,7 +231,7 @@ public class ProjectResetterTest { updateRef(repo2, metaConfig); } - verify(projectCache, only()).evict(project2); + verify(projectCache, only()).evictAndReindex(project2); } @Test @@ -248,7 +248,7 @@ public class ProjectResetterTest { createRef(repo2, RefNames.REFS_CONFIG); } - verify(projectCache, only()).evict(project2); + verify(projectCache, only()).evictAndReindex(project2); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java index 5279ba1be1..9b77b01745 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java @@ -71,7 +71,7 @@ public class AgreementsIT extends AbstractDaemonTest { config.updateProject( p -> p.setBooleanConfig(BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, value)); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); } } diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java index 6b2f4f14e4..9c74c486e7 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java @@ -1000,7 +1000,7 @@ public class ProjectIT extends AbstractDaemonTest { ProjectConfig config = projectConfigFactory.read(md); config.renameGroup(AccountGroup.uuid(group.id), newName); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); } Optional<String> afterRename = diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java index 415aa79558..c3bcbd3a5a 100644 --- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java @@ -226,7 +226,7 @@ public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest { ObjectId oldId = pc.getRevision(); ObjectId newId = pc.commit(md); assertThat(newId).isNotEqualTo(oldId); - projectCache.evict(pc.getProject()); + projectCache.evictAndReindex(pc.getProject()); } } diff --git a/javatests/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java b/javatests/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java index ef5e7dcd77..ab8e4d8af7 100644 --- a/javatests/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/config/CacheOperationsIT.java @@ -25,7 +25,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.common.data.GlobalCapability; -import com.google.gerrit.server.restapi.config.ListCaches.CacheInfo; +import com.google.gerrit.server.cache.CacheInfo; import com.google.gerrit.server.restapi.config.PostCaches; import com.google.inject.Inject; import java.util.Arrays; diff --git a/javatests/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java b/javatests/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java index a161ec4eb7..164f683f22 100644 --- a/javatests/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/config/FlushCacheIT.java @@ -23,7 +23,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.common.data.GlobalCapability; -import com.google.gerrit.server.restapi.config.ListCaches.CacheInfo; +import com.google.gerrit.server.cache.CacheInfo; import com.google.inject.Inject; import org.junit.Test; diff --git a/javatests/com/google/gerrit/acceptance/rest/config/GetCacheIT.java b/javatests/com/google/gerrit/acceptance/rest/config/GetCacheIT.java index 247d63b797..8765360423 100644 --- a/javatests/com/google/gerrit/acceptance/rest/config/GetCacheIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/config/GetCacheIT.java @@ -18,8 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; -import com.google.gerrit.server.restapi.config.ListCaches.CacheInfo; -import com.google.gerrit.server.restapi.config.ListCaches.CacheType; +import com.google.gerrit.server.cache.CacheInfo; import org.junit.Test; public class GetCacheIT extends AbstractDaemonTest { @@ -31,7 +30,7 @@ public class GetCacheIT extends AbstractDaemonTest { CacheInfo result = newGson().fromJson(r.getReader(), CacheInfo.class); assertThat(result.name).isEqualTo("accounts"); - assertThat(result.type).isEqualTo(CacheType.MEM); + assertThat(result.type).isEqualTo(CacheInfo.CacheType.MEM); assertThat(result.entries.mem).isAtLeast(1L); assertThat(result.averageGet).isNotNull(); assertThat(result.averageGet).endsWith("s"); diff --git a/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java b/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java index 8baeffc4aa..be21436864 100644 --- a/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/config/ListCachesIT.java @@ -21,8 +21,7 @@ import com.google.common.collect.Ordering; import com.google.common.io.BaseEncoding; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; -import com.google.gerrit.server.restapi.config.ListCaches.CacheInfo; -import com.google.gerrit.server.restapi.config.ListCaches.CacheType; +import com.google.gerrit.server.cache.CacheInfo; import com.google.gson.reflect.TypeToken; import java.util.Arrays; import java.util.List; @@ -40,7 +39,7 @@ public class ListCachesIT extends AbstractDaemonTest { assertThat(result).containsKey("accounts"); CacheInfo accountsCacheInfo = result.get("accounts"); - assertThat(accountsCacheInfo.type).isEqualTo(CacheType.MEM); + assertThat(accountsCacheInfo.type).isEqualTo(CacheInfo.CacheType.MEM); assertThat(accountsCacheInfo.entries.mem).isAtLeast(1L); assertThat(accountsCacheInfo.averageGet).isNotNull(); assertThat(accountsCacheInfo.averageGet).endsWith("s"); diff --git a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java index 5e3be9ac2b..8f341aabec 100644 --- a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java +++ b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java @@ -205,7 +205,7 @@ public class LabelNormalizerTest { private void save(ProjectConfig pc) throws Exception { try (MetaDataUpdate md = metaDataUpdateFactory.create(pc.getProject().getNameKey(), user)) { pc.commit(md); - projectCache.evict(pc.getProject().getNameKey()); + projectCache.evictAndReindex(pc.getProject().getNameKey()); } } diff --git a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java index a23ccaba9a..fc56a3c00f 100644 --- a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java +++ b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java @@ -74,6 +74,11 @@ public class FakeChangeIndex implements ChangeIndex { } @Override + public void insert(ChangeData obj) { + throw new UnsupportedOperationException(); + } + + @Override public void replace(ChangeData cd) { throw new UnsupportedOperationException(); } diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 332548ad3b..9acce8f605 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1135,7 +1135,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { cfg.upsertLabelType(verified); cfg.commit(md); } - projectCache.evict(project); + projectCache.evictAndReindex(project); String heads = RefNames.REFS_HEADS + "*"; projectOperations @@ -2265,7 +2265,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { }); config.commit(md); - projectCache.evict(config.getProject()); + projectCache.evictAndReindex(config.getProject()); } } diff --git a/plugins/delete-project b/plugins/delete-project -Subproject 5dcb1a64e779def0309354dd0e9886189845b02 +Subproject 612f143792652d571ecfcb19915ad5754a3ba1a diff --git a/plugins/webhooks b/plugins/webhooks -Subproject 9e4cd708b96e4fab79b4101eaf3f3e6a8b872dc +Subproject 8df6303e652857c3d7cee3348d2cb7d40a7db1a diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts index 475a69ca41..dd2c8d3219 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts @@ -1608,7 +1608,7 @@ export class GrRestApiInterface params.n = n; } if (project) { - params.p = encodeURIComponent(project); + params.p = project; } return this._restApiHelper.fetchJSON({ url: '/groups/', |