commit ad6dd0bb6c5ebeafaa57204a04330df6658c3861
from: Omar Polo <op@omarpolo.com>
date: Tue Jul 11 16:10:14 2023 UTC

gotadmin cleanup: don't delete pack files that are too young

similar to what we do for loose objects, avoid deleting pack files
that were created "too soon" unless -a is given.  This prevents
races when gotadmin load, got fetch or gotd are installing a new
pack file and a concurrent gotadmin clean attempts to remove it.

ok stsp

commit - 77c65f86323fa18ae23ab5bb93c486a0c840f308
commit + ad6dd0bb6c5ebeafaa57204a04330df6658c3861
blob - a12b9a6780a79ce5cb1abda9df2ddeadfba7f34f
blob + ba7276ad6779e4b6a924ef8284b01c4bbdb79216
--- lib/repository_admin.c
+++ lib/repository_admin.c
@@ -1191,7 +1191,8 @@ done:
 
 static const struct got_error *
 purge_redundant_pack(struct got_repository *repo, const char *packidx_path,
-    int dry_run, int *remove, off_t *size_before, off_t *size_after)
+    int dry_run, int ignore_mtime, time_t max_mtime,
+    int *remove, off_t *size_before, off_t *size_after)
 {
 	static const char *ext[] = {".idx", ".pack", ".rev", ".bitmap",
 	    ".promisor", ".mtimes"};
@@ -1203,6 +1204,20 @@ purge_redundant_pack(struct got_repository *repo, cons
 		return got_error(GOT_ERR_NO_SPACE);
 
 	/*
+	 * Do not delete pack files which are younger than our maximum
+	 * modification time threshold.  This prevents a race where a
+	 * new pack file which is being added to the repository
+	 * concurrently would be deleted.
+	 */
+	if (fstatat(got_repo_get_fd(repo), path, &sb, 0) == -1) {
+		if (errno == ENOENT)
+			return NULL;
+		return got_error_from_errno2("fstatat", path);
+	}
+	if (!ignore_mtime && sb.st_mtime > max_mtime)
+		*remove = 0;
+
+	/*
 	 * For compatibility with Git, if a matching .keep file exist
 	 * don't delete the packfile.
 	 */
@@ -1308,8 +1323,9 @@ pack_info_cmp(const void *a, const void *b)
 static const struct got_error *
 repo_purge_redundant_packfiles(struct got_repository *repo,
     struct got_object_idset *traversed_ids,
-    off_t *size_before, off_t *size_after, int dry_run,
-    int nloose, int ncommits, int npurged, struct got_ratelimit *rl,
+    off_t *size_before, off_t *size_after, int dry_run, int ignore_mtime,
+    time_t max_mtime, int nloose, int ncommits, int npurged,
+    struct got_ratelimit *rl,
     got_cleanup_progress_cb progress_cb, void *progress_arg,
     got_cancel_cb cancel_cb, void *cancel_arg)
 {
@@ -1362,7 +1378,7 @@ repo_purge_redundant_packfiles(struct got_repository *
 		if (err)
 			goto done;
 		err = purge_redundant_pack(repo, sorted[i].path, dry_run,
-		    &remove, size_before, size_after);
+		    ignore_mtime, max_mtime, &remove, size_before, size_after);
 		if (err)
 			goto done;
 		if (!remove)
@@ -1467,8 +1483,9 @@ got_repo_cleanup(struct got_repository *repo,
 		goto done;
 
 	err = repo_purge_redundant_packfiles(repo, traversed_ids,
-	    pack_before, pack_after, dry_run, *nloose, *ncommits, npurged,
-	    &rl, progress_cb, progress_arg, cancel_cb, cancel_arg);
+	    pack_before, pack_after, dry_run, ignore_mtime, max_mtime,
+	    *nloose, *ncommits, npurged, &rl, progress_cb, progress_arg,
+	    cancel_cb, cancel_arg);
 	if (err)
 		goto done;
 
blob - 757a755adad5a2bca886449ea29bc283017dd221
blob + f4a50c06198ba6fd2e8397c44a51a8aaa92817d3
--- regress/cmdline/cleanup.sh
+++ regress/cmdline/cleanup.sh
@@ -274,7 +274,7 @@ test_cleanup_redundant_pack_files() {
 	(cd "$testroot/repo" && git checkout -q master)
 	(cd "$testroot/repo" && got branch -d tempbranch) >/dev/null
 
-	gotadmin cleanup -q -r "$testroot/repo"
+	gotadmin cleanup -a -q -r "$testroot/repo"
 	n=$(gotadmin info -r "$testroot/repo" | awk '/^pack files/{print $3}')
 	if [ "$n" -ne 2 ]; then
 		echo "expected 2 pack files left, $n found instead" >&2
@@ -301,7 +301,7 @@ test_cleanup_redundant_pack_files() {
 	done
 	gotadmin pack -r "$testroot/repo" >/dev/null
 
-	gotadmin cleanup -q -r "$testroot/repo"
+	gotadmin cleanup -a -q -r "$testroot/repo"
 
 	n=$(gotadmin info -r "$testroot/repo" | awk '/^pack files/{print $3}')
 	if [ "$n" -ne 3 ]; then
@@ -324,7 +324,7 @@ test_cleanup_redundant_pack_files() {
 
 	gotadmin pack -a -x master -r "$testroot/repo" >/dev/null
 
-	gotadmin cleanup -q -r "$testroot/repo"
+	gotadmin cleanup -a -q -r "$testroot/repo"
 	n=$(gotadmin info -r "$testroot/repo" | awk '/^pack files/{print $3}')
 	if [ "$n" -ne 3 ]; then
 		echo "expected 3 pack files left, $n found instead" >&2