Merge branch 'stable-3.7' into stable-3.8
* stable-3.7:
Clarify constraints of project deletions
Introduce projectVersion cache
Rename getPathFromDynamoDB to getItemFromDynamoDB
Handle project deletion
Verify formatting using GJF 1.7
Change-Id: I268969751d31dcbefa858f1299b41aaf43982d1a
diff --git a/Jenkinsfile b/Jenkinsfile
index b1692ff..6ee14c4 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -1,3 +1,4 @@
pluginPipeline(formatCheckId: 'gerritforge:plugins-aws-dynamodb-refdb-code-style',
buildCheckId: 'gerritforge:plugins-aws-dynamodb-refdb-build-test',
- extraModules: [ 'global-refdb' ])
+ extraModules: [ 'global-refdb' ],
+ gjfVersion: '1.7')
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);
}
