Merge "Persist group cache by uuid"

Documentation/config-gerrit.txt

@@ -815,6 +815,7 @@ Default is 1024 for most caches, except:
 * `"groups"`: default is unlimited
 * `"groups_byname"`: default is unlimited
 * `"groups_byuuid"`: default is unlimited
+* `"groups_byuuid_persisted"`: default is `1g` (1 GiB of disk space)
 * `"plugin_resources"`: default is 2m (2 MiB of memory)
 
 +
@@ -1038,6 +1039,17 @@ unlimited.
 External group membership obtained from LDAP is cached under
 `"ldap_groups"`.
 
+cache `"groups_byuuid_persisted"`::
++
+Caches the basic group information of internal groups by group UUID,
+including the group owner, name, and description.
++
+This is the persisted version of `groups_byuuid` cache. The intention of this
+cache is to have an in-memory size of 0.
++
+External group membership obtained from LDAP is cached under
+`"ldap_groups"`.
+
 cache `"groups_bymember"`::
 +
 Caches the groups which contain a specific member (account). If direct

java/com/google/gerrit/server/account/GroupCacheImpl.java

@@ -19,7 +19,16 @@ import com.google.common.cache.LoadingCache;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.entities.AccountGroup;
 import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.proto.Protos;
 import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.proto.Cache;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import com.google.gerrit.server.cache.serialize.ProtobufSerializer;
+import com.google.gerrit.server.cache.serialize.entities.InternalGroupSerializer;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.logging.Metadata;
 import com.google.gerrit.server.logging.TraceContext;
@@ -33,6 +42,10 @@ import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
+import org.bouncycastle.util.Strings;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
 
 /** Tracks group objects in memory for efficient access. */
 @Singleton
@@ -42,6 +55,7 @@ public class GroupCacheImpl implements GroupCache {
   private static final String BYID_NAME = "groups";
   private static final String BYNAME_NAME = "groups_byname";
   private static final String BYUUID_NAME = "groups_byuuid";
+  private static final String BYUUID_NAME_PERSISTED = "groups_byuuid_persisted";
 
   public static Module module() {
     return new CacheModule() {
@@ -55,9 +69,35 @@ public class GroupCacheImpl implements GroupCache {
             .maximumWeight(Long.MAX_VALUE)
             .loader(ByNameLoader.class);
 
+        // We split the group cache into two parts for performance reasons:
+        // 1) An in-memory part that has only the group ref uuid as key.
+        // 2) A persisted part that has the group ref uuid and sha1 of the ref as key.
+        //
+        // When loading dashboards or returning change query results we potentially
+        // need to access many groups.
+        // We want the persisted cache to be immutable and we want it to be impossible that a
+        // value for a given key is out of date. We therefore require the sha-1 in the key. That
+        // is in line with the rest of the caches in Gerrit.
+        //
+        // Splitting the cache into two chunks internally in this class allows us to retain
+        // the existing performance guarantees of not requiring reads for the repo for values
+        // cached in-memory but also to persist the cache which leads to a much improved
+        // cold-start behavior and in-memory miss latency.
+
         cache(BYUUID_NAME, String.class, new TypeLiteral<Optional<InternalGroup>>() {})
             .maximumWeight(Long.MAX_VALUE)
-            .loader(ByUUIDLoader.class);
+            .loader(ByUUIDInMemoryLoader.class);
+
+        persist(
+                BYUUID_NAME_PERSISTED,
+                Cache.GroupKeyProto.class,
+                new TypeLiteral<InternalGroup>() {})
+            .loader(PersistedByUUIDLoader.class)
+            .keySerializer(new ProtobufSerializer<>(Cache.GroupKeyProto.parser()))
+            .valueSerializer(PersistedInternalGroupSerializer.INSTANCE)
+            .diskLimit(1 << 30) // 1 GiB
+            .version(1)
+            .maximumWeight(0);
 
         bind(GroupCacheImpl.class);
         bind(GroupCache.class).to(GroupCacheImpl.class);
@@ -150,7 +190,7 @@ public class GroupCacheImpl implements GroupCache {
 
     @Override
     public Optional<InternalGroup> load(AccountGroup.Id key) throws Exception {
-      try (TraceTimer timer =
+      try (TraceTimer ignored =
           TraceContext.newTimer(
               "Loading group by ID", Metadata.builder().groupId(key.get()).build())) {
         return groupQueryProvider.get().byId(key);
@@ -168,7 +208,7 @@ public class GroupCacheImpl implements GroupCache {
 
     @Override
     public Optional<InternalGroup> load(String name) throws Exception {
-      try (TraceTimer timer =
+      try (TraceTimer ignored =
           TraceContext.newTimer(
               "Loading group by name", Metadata.builder().groupName(name).build())) {
         return groupQueryProvider.get().byName(AccountGroup.nameKey(name));
@@ -176,21 +216,89 @@ public class GroupCacheImpl implements GroupCache {
     }
   }
 
-  static class ByUUIDLoader extends CacheLoader<String, Optional<InternalGroup>> {
+  static class ByUUIDInMemoryLoader extends CacheLoader<String, Optional<InternalGroup>> {
-    private final Groups groups;
+    private final LoadingCache<Cache.GroupKeyProto, InternalGroup> persistedCache;
+    private final GitRepositoryManager repoManager;
+    private final AllUsersName allUsersName;
 
     @Inject
-    ByUUIDLoader(Groups groups) {
+    ByUUIDInMemoryLoader(
-      this.groups = groups;
+        @Named(BYUUID_NAME_PERSISTED)
+            LoadingCache<Cache.GroupKeyProto, InternalGroup> persistedCache,
+        GitRepositoryManager repoManager,
+        AllUsersName allUsersName) {
+      this.persistedCache = persistedCache;
+      this.repoManager = repoManager;
+      this.allUsersName = allUsersName;
     }
 
     @Override
     public Optional<InternalGroup> load(String uuid) throws Exception {
-      try (TraceTimer timer =
+      try (TraceTimer ignored =
-          TraceContext.newTimer(
+              TraceContext.newTimer(
-              "Loading group by UUID", Metadata.builder().groupUuid(uuid).build())) {
+                  "Loading group from serialized cache",
-        return groups.getGroup(AccountGroup.uuid(uuid));
+                  Metadata.builder().groupUuid(uuid).build());
+          Repository allUsers = repoManager.openRepository(allUsersName)) {
+        String ref = RefNames.refsGroups(AccountGroup.uuid(uuid));
+        Ref sha1 = allUsers.exactRef(ref);
+        if (sha1 == null) {
+          return Optional.empty();
+        }
+        Cache.GroupKeyProto key =
+            Cache.GroupKeyProto.newBuilder()
+                .setUuid(uuid)
+                .setRevision(ObjectIdConverter.create().toByteString(sha1.getObjectId()))
+                .build();
+        return Optional.of(persistedCache.get(key));
       }
     }
   }
+
+  static class PersistedByUUIDLoader extends CacheLoader<Cache.GroupKeyProto, InternalGroup> {
+    private final Groups groups;
+
+    @Inject
+    PersistedByUUIDLoader(Groups groups) {
+      this.groups = groups;
+    }
+
+    @Override
+    public InternalGroup load(Cache.GroupKeyProto key) throws Exception {
+      try (TraceTimer ignored =
+          TraceContext.newTimer(
+              "Loading group by UUID", Metadata.builder().groupUuid(key.getUuid()).build())) {
+        ObjectId sha1 = ObjectIdConverter.create().fromByteString(key.getRevision());
+        Optional<InternalGroup> loadedGroup =
+            groups.getGroup(AccountGroup.uuid(key.getUuid()), sha1);
+        if (!loadedGroup.isPresent()) {
+          throw new IllegalStateException(
+              String.format(
+                  "group %s should have the sha-1 %s, but " + "it was not found",
+                  key.getUuid(), sha1.getName()));
+        }
+        return loadedGroup.get();
+      }
+    }
+  }
+
+  private enum PersistedInternalGroupSerializer implements CacheSerializer<InternalGroup> {
+    INSTANCE;
+
+    @Override
+    public byte[] serialize(InternalGroup value) {
+      if (value == null) {
+        return new byte[0];
+      }
+      return Protos.toByteArray(InternalGroupSerializer.serialize(value));
+    }
+
+    @Override
+    public InternalGroup deserialize(byte[] in) {
+      if (Strings.fromByteArray(in).isEmpty()) {
+        return null;
+      }
+      return InternalGroupSerializer.deserialize(
+          Protos.parseUnchecked(Cache.InternalGroupProto.parser(), in));
+    }
+  }
 }

java/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializer.java

@@ -0,0 +1,82 @@
+// 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.serialize.entities;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.server.cache.proto.Cache;
+import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
+import java.sql.Timestamp;
+
+/** Helper to (de)serialize values for caches. */
+public class InternalGroupSerializer {
+  public static InternalGroup deserialize(Cache.InternalGroupProto proto) {
+    InternalGroup.Builder builder =
+        InternalGroup.builder()
+            .setId(AccountGroup.id(proto.getId()))
+            .setNameKey(AccountGroup.nameKey(proto.getName()))
+            .setOwnerGroupUUID(AccountGroup.uuid(proto.getOwnerGroupUuid()))
+            .setVisibleToAll(proto.getIsVisibleToAll())
+            .setGroupUUID(AccountGroup.uuid(proto.getGroupUuid()))
+            .setCreatedOn(new Timestamp(proto.getCreatedOn()))
+            .setMembers(
+                proto.getMembersIdsList().stream()
+                    .map(a -> Account.id(a))
+                    .collect(toImmutableSet()))
+            .setSubgroups(
+                proto.getSubgroupUuidsList().stream()
+                    .map(s -> AccountGroup.uuid(s))
+                    .collect(toImmutableSet()));
+
+    if (!proto.getDescription().isEmpty()) {
+      builder.setDescription(proto.getDescription());
+    }
+
+    if (!proto.getRefState().isEmpty()) {
+      builder.setRefState(ObjectIdConverter.create().fromByteString(proto.getRefState()));
+    }
+
+    return builder.build();
+  }
+
+  public static Cache.InternalGroupProto serialize(InternalGroup autoValue) {
+    Cache.InternalGroupProto.Builder builder =
+        Cache.InternalGroupProto.newBuilder()
+            .setId(autoValue.getId().get())
+            .setName(autoValue.getName())
+            .setOwnerGroupUuid(autoValue.getOwnerGroupUUID().get())
+            .setIsVisibleToAll(autoValue.isVisibleToAll())
+            .setGroupUuid(autoValue.getGroupUUID().get())
+            .setCreatedOn(autoValue.getCreatedOn().getTime());
+
+    autoValue.getMembers().stream().forEach(m -> builder.addMembersIds(m.get()));
+    autoValue.getSubgroups().stream().forEach(s -> builder.addSubgroupUuids(s.get()));
+
+    if (autoValue.getDescription() != null) {
+      builder.setDescription(autoValue.getDescription());
+    }
+
+    if (autoValue.getRefState() != null) {
+      builder.setRefState(ObjectIdConverter.create().toByteString(autoValue.getRefState()));
+    }
+
+    return builder.build();
+  }
+
+  private InternalGroupSerializer() {}
+}

java/com/google/gerrit/server/group/db/GroupConfig.java

@@ -24,6 +24,7 @@ import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.AccountGroup;
 import com.google.gerrit.entities.InternalGroup;
@@ -156,7 +157,7 @@ public class GroupConfig extends VersionedMetaData {
       Project.NameKey projectName,
       Repository repository,
       AccountGroup.UUID groupUuid,
-      ObjectId groupRefObjectId)
+      @Nullable ObjectId groupRefObjectId)
       throws IOException, ConfigInvalidException {
     GroupConfig groupConfig = new GroupConfig(groupUuid);
     if (groupRefObjectId == null) {

java/com/google/gerrit/server/group/db/Groups.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.group.db;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.AccountGroup;
 import com.google.gerrit.entities.AccountGroupByIdAudit;
 import com.google.gerrit.entities.AccountGroupMemberAudit;
@@ -79,6 +80,23 @@ public class Groups {
     }
   }
 
+  /**
+   * Returns the {@code InternalGroup} for the specified UUID and groupRefObjectId
+   *
+   * @param groupUuid the UUID of the group
+   * @param groupRefObjectId the ref revision of this group
+   * @return the found {@code InternalGroup} if it exists, or else an empty {@code Optional}
+   * @throws IOException if the group couldn't be retrieved from NoteDb
+   * @throws ConfigInvalidException if the group couldn't be retrieved from NoteDb
+   */
+  public Optional<InternalGroup> getGroup(
+      AccountGroup.UUID groupUuid, @Nullable ObjectId groupRefObjectId)
+      throws IOException, ConfigInvalidException {
+    try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
+      return getGroupFromNoteDb(allUsersName, allUsersRepo, groupUuid, groupRefObjectId);
+    }
+  }
+
   /**
    * Loads an internal group from NoteDb using the group UUID. This method returns the latest state
    * of the internal group.
@@ -97,7 +115,7 @@ public class Groups {
       AllUsersName allUsersName,
       Repository allUsersRepository,
       AccountGroup.UUID uuid,
-      ObjectId groupRefObjectId)
+      @Nullable ObjectId groupRefObjectId)
       throws IOException, ConfigInvalidException {
     GroupConfig groupConfig =
         GroupConfig.loadForGroup(allUsersName, allUsersRepository, uuid, groupRefObjectId);

javatests/com/google/gerrit/server/cache/serialize/entities/InternalGroupSerializerTest.java

@@ -0,0 +1,61 @@
+// 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.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.cache.serialize.entities.InternalGroupSerializer.deserialize;
+import static com.google.gerrit.server.cache.serialize.entities.InternalGroupSerializer.serialize;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.InternalGroup;
+import com.google.gerrit.server.util.time.TimeUtil;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class InternalGroupSerializerTest {
+  static final InternalGroup MINIMAL_VALUES_SET =
+      InternalGroup.builder()
+          .setId(AccountGroup.id(123456))
+          .setNameKey(AccountGroup.nameKey("group name"))
+          .setOwnerGroupUUID(AccountGroup.uuid("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"))
+          .setVisibleToAll(false)
+          .setGroupUUID(AccountGroup.uuid("deadbeefdeadbeefdeadbeefdeadbeef12345678"))
+          .setCreatedOn(TimeUtil.nowTs())
+          .setMembers(ImmutableSet.of(Account.id(123), Account.id(321)))
+          .setSubgroups(
+              ImmutableSet.of(
+                  AccountGroup.uuid("87654321deadbeefdeadbeefdeadbeefdeadbeef"),
+                  AccountGroup.uuid("deadbeefdeadbeefdeadbeefdeadbeef87654321")))
+          .build();
+
+  static final InternalGroup ALL_VALUES_SET =
+      MINIMAL_VALUES_SET
+          .toBuilder()
+          .setDescription("description")
+          .setRefState(ObjectId.fromString("12345678deadbeefdeadbeefdeadbeefdeadbeef"))
+          .build();
+
+  @Test
+  public void roundTrip() {
+    assertThat(deserialize(serialize(ALL_VALUES_SET))).isEqualTo(ALL_VALUES_SET);
+  }
+
+  @Test
+  public void roundTripWithMinimalValues() {
+    assertThat(deserialize(serialize(MINIMAL_VALUES_SET))).isEqualTo(MINIMAL_VALUES_SET);
+  }
+}

proto/cache.proto

@@ -286,6 +286,29 @@ message AllExternalGroupsProto {
   repeated ExternalGroupProto external_group = 1;
 }
 
+// Serialized key for com.google.gerrit.server.account.GroupCacheImpl.
+// Next ID: 3
+message GroupKeyProto {
+  string uuid = 1;
+  bytes revision = 2;
+}
+
+
+// Serialized form of com.google.gerrit.entities.InternalGroup.
+// Next ID: 11
+message InternalGroupProto {
+  int32 id = 1;
+  string name = 2;
+  string description = 3;
+  string owner_group_uuid = 4;
+  bool is_visible_to_all = 5;
+  string group_uuid = 6;
+  int64 created_on = 7;
+  repeated int32 members_ids = 8;
+  repeated string subgroup_uuids = 9;
+  bytes ref_state = 10;
+}
+
 // Key for com.google.gerrit.server.git.PureRevertCache.
 // Next ID: 4
 message PureRevertKeyProto {