opendev/gerrit
Code Issues Proposed changes

Merge "Optionally persist ExternalIdCache"

Browse Source
This commit is contained in:
Dave Borowitz
2018-08-27 15:01:42 +00:00
committed by Gerrit Code Review
parent 5561068d4f b20d23a8cd
commit bafd9a8dcd
5 changed files with 231 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
Documentation/config-gerrit.txt
View File

@@ -786,6 +786,7 @@ Default is 128 MiB per cache, except:
+
* `"change_notes"`: disk storage is disabled by default
* `"diff_summary"`: default is `1g` (1 GiB of disk space)
* `"external_ids_map"`: disk storage is disabled by default
+
If 0 or negative, disk storage for the cache is disabled.
@@ -862,8 +863,10 @@ of link:config-accounts.html#external-ids[all current external IDs]. The
cache may temporarily contain 2 entries, but the second one is promptly
expired.
+
It is not recommended to change the attributes of this cache away from
the defaults.
It is not recommended to change the in-memory attributes of this cache
away from the defaults. The cache may be persisted by setting
`diskLimit`, which is only recommended if cold start performance is
problematic.
cache `"git_tags"`::
+

61
java/com/google/gerrit/server/account/externalids/AllExternalIds.java
View File

@@ -15,12 +15,18 @@
package com.google.gerrit.server.account.externalids;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
import static java.util.stream.Collectors.toList;
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.SetMultimap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto;
import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto.ExternalIdProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
import com.google.gerrit.server.cache.serialize.ProtoCacheSerializers;
import com.google.gerrit.server.cache.serialize.ProtoCacheSerializers.ObjectIdConverter;
import java.util.Collection;
/** Cache value containing all external IDs. */
@@ -48,4 +54,59 @@ public abstract class AllExternalIds {
public abstract ImmutableSetMultimap<Account.Id, ExternalId> byAccount();
public abstract ImmutableSetMultimap<String, ExternalId> byEmail();
enum Serializer implements CacheSerializer<AllExternalIds> {
INSTANCE;
@Override
public byte[] serialize(AllExternalIds object) {
ObjectIdConverter idConverter = ObjectIdConverter.create();
AllExternalIdsProto.Builder allBuilder = AllExternalIdsProto.newBuilder();
object
.byAccount()
.values()
.stream()
.map(extId -> toProto(idConverter, extId))
.forEach(allBuilder::addExternalId);
return ProtoCacheSerializers.toByteArray(allBuilder.build());
}
private static ExternalIdProto toProto(ObjectIdConverter idConverter, ExternalId externalId) {
ExternalIdProto.Builder b =
ExternalIdProto.newBuilder()
.setKey(externalId.key().get())
.setAccountId(externalId.accountId().get());
if (externalId.email() != null) {
b.setEmail(externalId.email());
}
if (externalId.password() != null) {
b.setPassword(externalId.password());
}
if (externalId.blobId() != null) {
b.setBlobId(idConverter.toByteString(externalId.blobId()));
}
return b.build();
}
@Override
public AllExternalIds deserialize(byte[] in) {
ObjectIdConverter idConverter = ObjectIdConverter.create();
return create(
ProtoCacheSerializers.parseUnchecked(AllExternalIdsProto.parser(), in)
.getExternalIdList()
.stream()
.map(proto -> toExternalId(idConverter, proto))
.collect(toList()));
}
private static ExternalId toExternalId(ObjectIdConverter idConverter, ExternalIdProto proto) {
return ExternalId.create(
ExternalId.Key.parse(proto.getKey()),
new Account.Id(proto.getAccountId()),
// ExternalId treats null and empty strings the same, so no need to distinguish here.
proto.getEmail(),
proto.getPassword(),
!proto.getBlobId().isEmpty() ? idConverter.fromByteString(proto.getBlobId()) : null);
}
}
}

9
java/com/google/gerrit/server/account/externalids/ExternalIdModule.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.account.externalids;
import com.google.gerrit.server.account.externalids.ExternalIdCacheImpl.Loader;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
import com.google.inject.TypeLiteral;
import java.time.Duration;
import org.eclipse.jgit.lib.ObjectId;
@@ -23,7 +24,7 @@ import org.eclipse.jgit.lib.ObjectId;
public class ExternalIdModule extends CacheModule {
@Override
protected void configure() {
cache(ExternalIdCacheImpl.CACHE_NAME, ObjectId.class, new TypeLiteral<AllExternalIds>() {})
persist(ExternalIdCacheImpl.CACHE_NAME, ObjectId.class, new TypeLiteral<AllExternalIds>() {})
// The cached data is potentially pretty large and we are always only interested
// in the latest value. However, due to a race condition, it is possible for different
// threads to observe different values of the meta ref, and hence request different keys
@@ -32,7 +33,11 @@ public class ExternalIdModule extends CacheModule {
// memory.
.maximumWeight(2)
.expireFromMemoryAfterAccess(Duration.ofMinutes(1))
.loader(Loader.class);
.loader(Loader.class)
.diskLimit(-1)
.version(1)
.keySerializer(ObjectIdCacheSerializer.INSTANCE)
.valueSerializer(AllExternalIds.Serializer.INSTANCE);
bind(ExternalIdCacheImpl.class);
bind(ExternalIdCache.class).to(ExternalIdCacheImpl.class);

143
javatests/com/google/gerrit/server/account/externalids/AllExternalIdsTest.java Normal file
View File

@@ -0,0 +1,143 @@
// Copyright (C) 2018 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.account.externalids;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.byteString;
import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.externalids.AllExternalIds.Serializer;
import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto;
import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto.ExternalIdProto;
import com.google.inject.TypeLiteral;
import java.util.Arrays;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
public class AllExternalIdsTest {
@Test
public void serializeEmptyExternalIds() throws Exception {
assertRoundTrip(allExternalIds(), AllExternalIdsProto.getDefaultInstance());
}
@Test
public void serializeMultipleExternalIds() throws Exception {
Account.Id accountId1 = new Account.Id(1001);
Account.Id accountId2 = new Account.Id(1002);
assertRoundTrip(
allExternalIds(
ExternalId.create("scheme1", "id1", accountId1),
ExternalId.create("scheme2", "id2", accountId1),
ExternalId.create("scheme2", "id3", accountId2),
ExternalId.create("scheme3", "id4", accountId2)),
AllExternalIdsProto.newBuilder()
.addExternalId(
ExternalIdProto.newBuilder().setKey("scheme1:id1").setAccountId(1001).build())
.addExternalId(
ExternalIdProto.newBuilder().setKey("scheme2:id2").setAccountId(1001).build())
.addExternalId(
ExternalIdProto.newBuilder().setKey("scheme2:id3").setAccountId(1002).build())
.addExternalId(
ExternalIdProto.newBuilder().setKey("scheme3:id4").setAccountId(1002).build())
.build());
}
@Test
public void serializeExternalIdWithEmail() throws Exception {
assertRoundTrip(
allExternalIds(ExternalId.createEmail(new Account.Id(1001), "foo@example.com")),
AllExternalIdsProto.newBuilder()
.addExternalId(
ExternalIdProto.newBuilder()
.setKey("mailto:foo@example.com")
.setAccountId(1001)
.setEmail("foo@example.com"))
.build());
}
@Test
public void serializeExternalIdWithPassword() throws Exception {
assertRoundTrip(
allExternalIds(
ExternalId.create("scheme", "id", new Account.Id(1001), null, "hashed password")),
AllExternalIdsProto.newBuilder()
.addExternalId(
ExternalIdProto.newBuilder()
.setKey("scheme:id")
.setAccountId(1001)
.setPassword("hashed password"))
.build());
}
@Test
public void serializeExternalIdWithBlobId() throws Exception {
assertRoundTrip(
allExternalIds(
ExternalId.create(
ExternalId.create("scheme", "id", new Account.Id(1001)),
ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"))),
AllExternalIdsProto.newBuilder()
.addExternalId(
ExternalIdProto.newBuilder()
.setKey("scheme:id")
.setAccountId(1001)
.setBlobId(
byteString(
0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef)))
.build());
}
@Test
public void allExternalIdsMethods() {
assertThatSerializedClass(AllExternalIds.class)
.hasAutoValueMethods(
ImmutableMap.of(
"byAccount",
new TypeLiteral<ImmutableSetMultimap<Account.Id, ExternalId>>() {}.getType(),
"byEmail",
new TypeLiteral<ImmutableSetMultimap<String, ExternalId>>() {}.getType()));
}
@Test
public void externalIdMethods() {
assertThatSerializedClass(ExternalId.class)
.hasAutoValueMethods(
ImmutableMap.of(
"key", ExternalId.Key.class,
"accountId", Account.Id.class,
"email", String.class,
"password", String.class,
"blobId", ObjectId.class));
}
private static AllExternalIds allExternalIds(ExternalId... externalIds) {
return AllExternalIds.create(Arrays.asList(externalIds));
}
private static void assertRoundTrip(
AllExternalIds allExternalIds, AllExternalIdsProto expectedProto) throws Exception {
AllExternalIdsProto actualProto =
AllExternalIdsProto.parseFrom(Serializer.INSTANCE.serialize(allExternalIds));
assertThat(actualProto).ignoringRepeatedFieldOrder().isEqualTo(expectedProto);
AllExternalIds actual =
Serializer.INSTANCE.deserialize(Serializer.INSTANCE.serialize(allExternalIds));
assertThat(actual).isEqualTo(allExternalIds);
}
}

15
proto/cache.proto
View File

@@ -219,3 +219,18 @@ message TagSetHolderProto {
}
TagSetProto tags = 2;
}
// Serialized form of
// com.google.gerrit.server.account.externalids.AllExternalIds.
// Next ID: 2
message AllExternalIdsProto {
// Next ID: 6
message ExternalIdProto {
string key = 1;
int32 accountId = 2;
string email = 3;
string password = 4;
bytes blobId = 5;
}
repeated ExternalIdProto external_id = 1;
}