Move projects to delete into separate folder
Currently when projects are deleted they sometimes leave behind spurious
files, that, for one reason or another(i.e. File Handle still in use)
could not be deleted at the time the command run.
This is expected, so much so that, on a regular, configurable interval,
we scan for left over repositories to delete in Gerrit's git data folder
for any of these leftover folders.
If the installation has thousands of repositories, scanning the whole
git data folder can become very I/O intensive occupying precious
resources.
Move folders to be deleted to a configurable directory within the git
data folder, so that we only need to scan this, rather than the whole
git data folder.
Bug: Issue 461414275
Change-Id: Iefdd54f1e2a8f2c97477a1ab4b9d87779c0255b9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java
index e812021..c93ad77 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Configuration.java
@@ -46,6 +46,7 @@
private static final String DELETED_PROJECTS_PARENT = "Deleted-Projects";
private static final long DEFAULT_ARCHIVE_DURATION_DAYS = 180;
protected static final long DEFAULT_TRASH_FOLDER_MAX_ALLOWED_TIME_MINUTES = 10;
+ public static final String DEFAULT_TRASH_FOLDER_NAME = "";
private final boolean allowDeletionWithTags;
private final boolean archiveDeletedRepos;
@@ -53,6 +54,7 @@
private final long deleteArchivedReposAfter;
private final long deleteTrashFoldersMaxAllowedTime;
private final String deletedProjectsParent;
+ private final String trashFolderName;
private final Path archiveFolder;
private final List<Pattern> protectedProjects;
private final Optional<ScheduleConfig.Schedule> schedule;
@@ -91,6 +93,11 @@
.setKeyStartTime("deleteTrashFolderStartTime")
.setKeyJitter("deleteTrashFolderJitter")
.buildSchedule();
+ this.trashFolderName = cfg.getString("trashFolderName", DEFAULT_TRASH_FOLDER_NAME);
+ }
+
+ public String getTrashFolderName() {
+ return trashFolderName;
}
public long getDeleteTrashFoldersMaxAllowedTime() {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java
index 85ce8f5..550b12f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFolders.java
@@ -92,6 +92,7 @@
private ScheduledFuture<?> threadCompleted;
private final Optional<ScheduleConfig.Schedule> schedule;
private final long deleteTrashFoldersMaxAllowedTime;
+ private final String trashFolderName;
@Inject
public DeleteTrashFolders(
@@ -105,6 +106,7 @@
repoFolders.add(site.resolve(cfg.getString("gerrit", null, "basePath")));
repoFolders.addAll(repositoryCfg.getAllBasePaths());
schedule = pluginCfg.getSchedule();
+ trashFolderName = pluginCfg.getTrashFolderName();
deleteTrashFoldersMaxAllowedTime = pluginCfg.getDeleteTrashFoldersMaxAllowedTime();
this.workQueue = workQueue;
this.pluginName = pluginName;
@@ -146,10 +148,11 @@
private void evaluateIfTrashWithTimeLimit() {
Stopwatch stopWatch = Stopwatch.createStarted();
for (Path folder : repoFolders) {
- if (exceededMaxAllowedTime(folder, stopWatch)) {
+ Path deletedRepoFolder = folder.resolve(trashFolderName);
+ if (exceededMaxAllowedTime(deletedRepoFolder, stopWatch)) {
break;
}
- evaluateIfTrash(folder, stopWatch);
+ evaluateIfTrash(deletedRepoFolder, stopWatch);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDelete.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDelete.java
index c59f3fc..9b30188 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDelete.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDelete.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.events.ProjectDeletedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.googlesource.gerrit.plugins.deleteproject.Configuration;
import com.googlesource.gerrit.plugins.deleteproject.TimeMachine;
import java.io.File;
import java.io.IOException;
@@ -47,10 +48,12 @@
public class RepositoryDelete {
private final GitRepositoryManager repoManager;
+ private final Configuration configuration;
@Inject
- public RepositoryDelete(GitRepositoryManager repoManager) {
+ public RepositoryDelete(GitRepositoryManager repoManager, Configuration configuration) {
this.repoManager = repoManager;
+ this.configuration = configuration;
}
private static final FluentLogger log = FluentLogger.forEnclosingClass();
@@ -94,7 +97,8 @@
if (archiveDeletedRepos) {
archiveGitRepository(projectName, repoPath, archivedFolder, deletedListeners);
} else {
- deleteGitRepository(projectName, repoPath, deletedListeners);
+ deleteGitRepository(
+ projectName, repoPath, deletedListeners, configuration.getTrashFolderName());
}
}
}
@@ -147,11 +151,15 @@
}
private static void deleteGitRepository(
- String projectName, Path repoPath, DynamicSet<ProjectDeletedListener> deletedListeners)
+ String projectName,
+ Path repoPath,
+ DynamicSet<ProjectDeletedListener> deletedListeners,
+ String trashFolderName)
throws IOException {
// Delete the repository from disk
Path basePath = getBasePath(repoPath, projectName);
- Path trash = renameRepository(repoPath, basePath, projectName, "deleted");
+ Path trash =
+ moveRepositoryForDeletion(repoPath, basePath, projectName, "deleted", trashFolderName);
try {
MoreFiles.deleteRecursively(trash, ALLOW_INSECURE);
recursivelyDeleteEmptyParents(repoPath.toFile().getParentFile(), basePath.toFile());
@@ -174,9 +182,16 @@
Path newRepo =
basePath.resolve(
projectName + "." + FORMAT.format(TimeMachine.now()) + ".%" + option + "%.git");
+ Files.createDirectories(newRepo.getParent());
return Files.move(directory, newRepo, StandardCopyOption.ATOMIC_MOVE);
}
+ private static Path moveRepositoryForDeletion(
+ Path directory, Path basePath, String projectName, String option, String trashFolderName)
+ throws IOException {
+ return renameRepository(directory, basePath.resolve(trashFolderName), projectName, option);
+ }
+
/**
* Recursively delete the specified file and its parent files until we hit the file {@code Until}
* or the parent file is populated. This is used when we have a tree structure such as a/b/c/d.git
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index cae3bfe..d9b14e8 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -124,6 +124,15 @@
By default 10 minutes.
+plugin.@PLUGIN@.trashFolderName
+: Parent folder for all trash folders
+
+ Specifies a common location for all trash folders. When defined, it can
+ massively reduce I/O overhead by not scanning the whole git data folder
+ but rather a much smaller subset.
+
+ By default empty string
+
Delete Trash Folder Scheduling
=============
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java
index 4f6098e..06300a3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/DeleteTrashFoldersTest.java
@@ -77,6 +77,7 @@
when(repositoryCfg.getAllBasePaths()).thenReturn(ImmutableList.of());
when(workQueue.getDefaultQueue()).thenReturn(fakeScheduledExecutor);
when(pluginCfg.getDeleteTrashFoldersMaxAllowedTime()).thenReturn(10L);
+ when(pluginCfg.getTrashFolderName()).thenReturn("some-trash-folder");
trashFolders =
new DeleteTrashFolders(
sitePaths, cfg, repositoryCfg, pluginCfg, workQueue, DELETE_PROJECT_PLUGIN);
@@ -95,7 +96,7 @@
sitePaths, cfg, repositoryCfg, pluginCfg, workQueue, DELETE_PROJECT_PLUGIN);
trashFolders.start();
- try (FileRepository repoToDelete = createRepository(REPOSITORY_TO_DELETE)) {
+ try (FileRepository repoToDelete = createRepositoryToDelete(REPOSITORY_TO_DELETE)) {
// Repository is not deleted at 1/2 time of the initial delay
fakeScheduledExecutor.advance(
TimeUnit.MINUTES.toSeconds(INITIAL_DELAY_MIN / 2), TimeUnit.SECONDS);
@@ -107,7 +108,7 @@
assertThatRepositoryIsDeleted(repoToDelete);
}
- try (FileRepository repoToDelete = createRepository(REPOSITORY_TO_DELETE)) {
+ try (FileRepository repoToDelete = createRepositoryToDelete(REPOSITORY_TO_DELETE)) {
// Repository recreated
assertThatRepositoryExists(repoToDelete);
@@ -131,7 +132,7 @@
@Test
public void testStart() throws Exception {
- FileRepository repoToDelete = createRepository(REPOSITORY_TO_DELETE);
+ FileRepository repoToDelete = createRepositoryToDelete(REPOSITORY_TO_DELETE);
FileRepository repoToKeep = createRepository("anotherRepo.git");
trashFolders.start();
trashFolders.getWorkerFuture().get();
@@ -170,6 +171,11 @@
return (FileRepository) repository;
}
+ private FileRepository createRepositoryToDelete(String repoName) throws IOException {
+ return createRepository(
+ basePath.resolve(pluginCfg.getTrashFolderName()).resolve(repoName).toString());
+ }
+
private void setupTrashFolderCleanupSchedule(String startTime, String interval) {
cfg.setString("plugin", DELETE_PROJECT_PLUGIN, "deleteTrashFolderStartTime", startTime);
cfg.setString("plugin", DELETE_PROJECT_PLUGIN, "deleteTrashFolderInterval", interval);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDeleteTest.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDeleteTest.java
index 512b8b5..421cd65 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDeleteTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/fs/RepositoryDeleteTest.java
@@ -17,6 +17,7 @@
package com.googlesource.gerrit.plugins.deleteproject.fs;
import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.deleteproject.Configuration.DEFAULT_TRASH_FOLDER_NAME;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.when;
@@ -26,6 +27,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.googlesource.gerrit.plugins.deleteproject.Configuration;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
@@ -51,6 +53,7 @@
@Mock private GitRepositoryManager repoManager;
@Mock private ProjectDeletedListener projectDeleteListener;
+ @Mock private Configuration configMock;
@Rule public TemporaryFolder tempFolder = new TemporaryFolder();
@@ -64,6 +67,7 @@
deletedListeners = new DynamicSet<>();
handle = deletedListeners.add("testPlugin", projectDeleteListener);
basePath = tempFolder.newFolder().toPath().resolve("base");
+ when(configMock.getTrashFolderName()).thenReturn(DEFAULT_TRASH_FOLDER_NAME);
}
@Test
@@ -72,7 +76,19 @@
Repository repository = createRepository(repoName);
Project.NameKey nameKey = Project.nameKey(repoName);
when(repoManager.openRepository(nameKey)).thenReturn(repository);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
+ repositoryDelete.execute(nameKey);
+ assertThat(repository.getDirectory().exists()).isFalse();
+ }
+
+ @Test
+ public void shouldDeleteRepositoryEvenWithDifferentTrashFolderPath() throws Exception {
+ when(configMock.getTrashFolderName()).thenReturn(".%trash%");
+ String repoName = "testRepo";
+ Repository repository = createRepository(repoName);
+ Project.NameKey nameKey = Project.nameKey(repoName);
+ when(repoManager.openRepository(nameKey)).thenReturn(repository);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
repositoryDelete.execute(nameKey);
assertThat(repository.getDirectory().exists()).isFalse();
}
@@ -83,7 +99,7 @@
Repository repository = createRepository(repoName);
Project.NameKey nameKey = Project.nameKey(repoName);
when(repoManager.openRepository(nameKey)).thenReturn(repository);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
repositoryDelete.execute(nameKey);
assertThat(repository.getDirectory().exists()).isFalse();
}
@@ -98,7 +114,7 @@
Project.NameKey nameKey = Project.nameKey(repoToDeleteName);
when(repoManager.openRepository(nameKey)).thenReturn(repoToDelete);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
repositoryDelete.execute(nameKey);
assertThat(repoToDelete.getDirectory().exists()).isFalse();
assertThat(repoToKeep.getDirectory().exists()).isTrue();
@@ -110,7 +126,7 @@
Repository repository = createRepository(repoName);
Project.NameKey nameKey = Project.nameKey(repoName);
when(repoManager.openRepository(nameKey)).thenReturn(repository);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
repositoryDelete.execute(nameKey, true, false, NO_ARCHIVE_PATH, deletedListeners);
assertThat(repository.getDirectory().exists()).isTrue();
}
@@ -129,7 +145,7 @@
Path archiveFolder = basePath.resolve("test_archive");
Project.NameKey nameKey = Project.nameKey(repoName);
when(repoManager.openRepository(nameKey)).thenReturn(repository);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
repositoryDelete.execute(nameKey, false, true, Optional.of(archiveFolder), deletedListeners);
assertThat(repository.getDirectory().exists()).isFalse();
String patternToVerify = archiveFolder.resolve(repoName).toString() + "*%archived%.git";
@@ -142,7 +158,7 @@
Repository repository = createRepository(repoName);
Project.NameKey nameKey = Project.nameKey(repoName);
when(repoManager.openRepository(nameKey)).thenReturn(repository);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
repositoryDelete.execute(nameKey, false, false, NO_ARCHIVE_PATH, deletedListeners);
Mockito.verify(projectDeleteListener).onProjectDeleted(any());
}
@@ -153,7 +169,7 @@
Repository repository = createRepository(repoName);
Project.NameKey nameKey = Project.nameKey(repoName);
when(repoManager.openRepository(nameKey)).thenReturn(repository);
- repositoryDelete = new RepositoryDelete(repoManager);
+ repositoryDelete = new RepositoryDelete(repoManager, configMock);
handle.remove();
repositoryDelete.execute(nameKey, false, false, NO_ARCHIVE_PATH, deletedListeners);
Mockito.verify(projectDeleteListener, never()).onProjectDeleted(any());
