Merge branch 'stable-3.8' into stable-3.9
* stable-3.8:
Clarify constraints of project deletions
Introduce projectVersion cache
Rename getPathFromDynamoDB to getItemFromDynamoDB
Handle project deletion
Verify formatting using GJF 1.7
Note that since 3.9 formatting version is done with GJF 1.24, so the
"Verify formatting using GJF 1.7" commit was dropped.
Change-Id: I6e6fb91204da0bc267f453d1721a109f60f4c0f2
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
index 933fd3f..712f5d1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb;
+import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.ProjectVersionCacheModule.PROJECT_VERSION_CACHE;
+
import com.amazonaws.services.dynamodbv2.AcquireLockOptions;
import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
import com.amazonaws.services.dynamodbv2.AmazonDynamoDBLockClient;
@@ -28,12 +30,18 @@
import com.gerritforge.gerrit.globalrefdb.ExtendedGlobalRefDatabase;
import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException;
import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.Project.NameKey;
import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.name.Named;
import java.util.Optional;
+import java.util.concurrent.ExecutionException;
import javax.inject.Singleton;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -51,25 +59,36 @@
private final AmazonDynamoDBLockClient lockClient;
private final AmazonDynamoDB dynamoDBClient;
private final Configuration configuration;
+ private final LoadingCache<String, Optional<Integer>> projectVersionCache;
@Inject
DynamoDBRefDatabase(
AmazonDynamoDBLockClient lockClient,
AmazonDynamoDB dynamoDBClient,
- Configuration configuration) {
+ Configuration configuration,
+ @Named(PROJECT_VERSION_CACHE) LoadingCache<String, Optional<Integer>> projectVersionCache) {
this.lockClient = lockClient;
this.dynamoDBClient = dynamoDBClient;
this.configuration = configuration;
+ this.projectVersionCache = projectVersionCache;
}
- static String pathFor(Project.NameKey projectName, String refName) {
- return "/" + projectName + "/" + refName;
+ String pathFor(Project.NameKey projectName, String refName) {
+ return versionPrefix(getCurrentVersion(projectName)) + "/" + projectName + "/" + refName;
+ }
+
+ static String currentVersionKey(Project.NameKey projectName) {
+ return "|" + projectName;
+ }
+
+ static String versionPrefix(Integer version) {
+ return version != null ? "|" + version : "";
}
@Override
public boolean isUpToDate(Project.NameKey project, Ref ref) throws GlobalRefDbLockException {
try {
- GetItemResult result = getPathFromDynamoDB(project, ref.getName());
+ GetItemResult result = getItemFromDynamoDB(pathFor(project, ref.getName()));
if (!exists(result)) {
return true;
}
@@ -150,7 +169,10 @@
@Override
public <T> void put(NameKey project, String refName, T value) throws GlobalRefDbSystemError {
- String refPath = pathFor(project, refName);
+ doPut(project, pathFor(project, refName), value);
+ }
+
+ public <T> void doPut(NameKey project, String refPath, T value) throws GlobalRefDbSystemError {
String refValue =
Optional.ofNullable(value).map(Object::toString).orElse(ObjectId.zeroId().getName());
try {
@@ -197,7 +219,7 @@
@Override
public boolean exists(Project.NameKey project, String refName) {
try {
- if (!exists(getPathFromDynamoDB(project, refName))) {
+ if (!exists(getItemFromDynamoDB(pathFor(project, refName)))) {
logger.atFine().log("ref '%s' does not exist in dynamodb", pathFor(project, refName));
return false;
}
@@ -211,11 +233,24 @@
return false;
}
+ @Nullable
+ public Integer getCurrentVersion(Project.NameKey project) throws GlobalRefDbSystemError {
+ try {
+ return projectVersionCache.get(project.get()).orElse(null);
+ } catch (ExecutionException e) {
+ throw new GlobalRefDbSystemError("Could not check project version", e);
+ }
+ }
+
@Override
public void remove(Project.NameKey project) throws GlobalRefDbSystemError {
- // TODO: to remove all refs related to project we'd need to be able to query
- // dynamodb by 'project': perhaps we should have a composite key of:
- // PK: project, SK: ref
+ Integer currentVersion = getCurrentVersion(project);
+ int nextVersion = (currentVersion != null ? currentVersion : 0) + 1;
+
+ doPut(project, currentVersionKey(project), Integer.toString(nextVersion));
+ logger.atWarning().log(
+ "Project %s removed, current version %s, next version %s",
+ project, currentVersion, nextVersion);
}
@SuppressWarnings("unchecked")
@@ -223,7 +258,7 @@
public <T> Optional<T> get(Project.NameKey project, String refName, Class<T> clazz)
throws GlobalRefDbSystemError {
try {
- GetItemResult item = getPathFromDynamoDB(project, refName);
+ GetItemResult item = getItemFromDynamoDB(pathFor(project, refName));
if (!exists(item)) {
return Optional.empty();
}
@@ -239,14 +274,41 @@
}
}
- private GetItemResult getPathFromDynamoDB(Project.NameKey project, String refName) {
- return dynamoDBClient.getItem(
- configuration.getRefsDbTableName(),
- ImmutableMap.of(REF_DB_PRIMARY_KEY, new AttributeValue(pathFor(project, refName))),
- true);
+ private GetItemResult getItemFromDynamoDB(String refPath) {
+ return getItemFromDynamoDB(refPath, true);
}
- private boolean exists(GetItemResult result) {
+ public GetItemResult getItemFromDynamoDB(String refPath, Boolean consistentRead) {
+ return dynamoDBClient.getItem(
+ configuration.getRefsDbTableName(),
+ ImmutableMap.of(REF_DB_PRIMARY_KEY, new AttributeValue(refPath)),
+ consistentRead);
+ }
+
+ public boolean exists(GetItemResult result) {
return result.getItem() != null && !result.getItem().isEmpty();
}
+
+ static class ProjectVersionCacheLoader extends CacheLoader<String, Optional<Integer>> {
+
+ private final Provider<DynamoDBRefDatabase> dynamoDBRefDatabaseProvider;
+
+ @Inject
+ public ProjectVersionCacheLoader(Provider<DynamoDBRefDatabase> dynamoDBRefDatabaseProvider) {
+ this.dynamoDBRefDatabaseProvider = dynamoDBRefDatabaseProvider;
+ }
+
+ @Override
+ public Optional<Integer> load(String project) throws Exception {
+ GetItemResult item =
+ dynamoDBRefDatabaseProvider
+ .get()
+ .getItemFromDynamoDB(currentVersionKey(Project.nameKey(project)), false);
+ Integer currentVersion =
+ dynamoDBRefDatabaseProvider.get().exists(item)
+ ? Integer.parseInt(item.getItem().get(REF_DB_VALUE_KEY).getS())
+ : null;
+ return Optional.ofNullable(currentVersion);
+ }
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Module.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Module.java
index 946d401..4def9f6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Module.java
@@ -33,6 +33,7 @@
DynamicItem.bind(binder(), GlobalRefDatabase.class)
.to(DynamoDBRefDatabase.class)
.in(Scopes.SINGLETON);
+ install(new ProjectVersionCacheModule());
bind(AmazonDynamoDB.class).toProvider(AmazonDynamoDBProvider.class).in(SINGLETON);
bind(AmazonDynamoDBLockClient.class).toProvider(DynamoDBLockClientProvider.class).in(SINGLETON);
listener().to(DynamoDBLifeCycleManager.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ProjectVersionCacheModule.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ProjectVersionCacheModule.java
new file mode 100644
index 0000000..55c581c
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ProjectVersionCacheModule.java
@@ -0,0 +1,32 @@
+// Copyright (C) 2024 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.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb;
+
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.inject.TypeLiteral;
+import java.time.Duration;
+import java.util.Optional;
+
+public class ProjectVersionCacheModule extends CacheModule {
+
+ public static final String PROJECT_VERSION_CACHE = "projectVersion";
+
+ @Override
+ protected void configure() {
+ cache(PROJECT_VERSION_CACHE, String.class, new TypeLiteral<Optional<Integer>>() {})
+ .expireAfterWrite(Duration.ofSeconds(60))
+ .maximumWeight(Long.MAX_VALUE)
+ .loader(DynamoDBRefDatabase.ProjectVersionCacheLoader.class);
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 0d871cc..4f85f41 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -40,5 +40,19 @@
Default: When not specified credentials are provided via the Default Credentials
Provider Chain, as explained [here](https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/credentials.html)
+### Project Deletion
+This plugin introduces a caching mechanism to reduce the number of requests to
+DynamoDB by storing the current live version of a project.
+
+Checking the current live version of a project directly from DynamoDB can be
+costly. To minimise the number of DynamoDb operations that are unlikely to
+provide a different result, avoiding excessive charges for reading continuously
+unchanged data, the plugin employs a cache with a time-to-live (TTL) of `60
+seconds`.
+
+> **IMPORTANT NOTE** When a project is deleted, it cannot be recreated with the
+> same name for the subsequent 60 seconds, because of the of the _live version_
+> caching mechanism described above. Any attempt to recreate the same project
+> with the same name *before 60 seconds* will result in a failure.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
index 38b6dc8..ad82802 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
@@ -21,11 +21,15 @@
import static org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB;
import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
+import com.google.common.cache.LoadingCache;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.WaitUtil;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
+import com.google.inject.Key;
+import com.google.inject.TypeLiteral;
+import com.google.inject.name.Names;
import java.time.Duration;
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
@@ -241,6 +245,34 @@
assertThat(dynamoDBRefDatabase().compareAndPut(project, refName, null, newRefValue)).isTrue();
}
+ @Test
+ public void projectVersionShouldBeUsedAsPrefix() {
+ String versionKey = DynamoDBRefDatabase.currentVersionKey(project);
+ dynamoDBRefDatabase().doPut(project, versionKey, "1");
+
+ assertThat(dynamoDBRefDatabase().pathFor(project, "refs/heads/master"))
+ .isEqualTo("|1/" + project + "/refs/heads/master");
+ }
+
+ @Test
+ public void removeProjectShouldIncreaseProjectVersionWhenNotCached() {
+ assertThat(dynamoDBRefDatabase().getCurrentVersion(project)).isNull();
+
+ dynamoDBRefDatabase().remove(project);
+ projectVersionCache().invalidate(project.get());
+
+ assertThat(dynamoDBRefDatabase().getCurrentVersion(project)).isEqualTo(1);
+ }
+
+ @Test
+ public void removeProjectShouldKeepCurrentVersionWhenCached() {
+ assertThat(dynamoDBRefDatabase().getCurrentVersion(project)).isNull();
+
+ dynamoDBRefDatabase().remove(project);
+
+ assertThat(dynamoDBRefDatabase().getCurrentVersion(project)).isNull();
+ }
+
private AmazonDynamoDB dynamoDBClient() {
return plugin.getSysInjector().getInstance(AmazonDynamoDB.class);
}
@@ -249,6 +281,15 @@
return plugin.getSysInjector().getInstance(DynamoDBRefDatabase.class);
}
+ private LoadingCache<String, Optional<Integer>> projectVersionCache() {
+ return plugin
+ .getSysInjector()
+ .getInstance(
+ Key.get(
+ new TypeLiteral<LoadingCache<String, Optional<Integer>>>() {},
+ Names.named(ProjectVersionCacheModule.PROJECT_VERSION_CACHE)));
+ }
+
private void createRefInDynamoDB(Project.NameKey project, String refPath, String refValue) {
dynamoDBRefDatabase().put(project, refPath, refValue);
}
