commit 3bf0e21f50b11c683f08a06c8ab362fe220adc2b from: Omar Polo date: Mon Jun 19 13:33:27 2023 UTC add a lock for the cleanup operation This adds the functions got_repo_cleanup_prepare() and _complete() to lock/unlock the repository to prevent multiple `gotadmin cleanup' or `git gc' operations to run in parallel. improvements and ok stsp@ commit - 664d62f9f2469ee2bc583ab0bbc90e56bf2d560e commit + 3bf0e21f50b11c683f08a06c8ab362fe220adc2b blob - b374606457024caef32ca71ee66bd3057552e4b1 blob + 2b43e7ed974e7ba95556eac4c0b718e3c5706bb3 --- gotadmin/gotadmin.c +++ gotadmin/gotadmin.c @@ -1236,6 +1236,7 @@ cmd_cleanup(int argc, char *argv[]) struct got_repository *repo = NULL; int ch, dry_run = 0, npacked = 0, verbosity = 0; int remove_lonely_packidx = 0, ignore_mtime = 0; + struct got_lockfile *lock = NULL; struct got_cleanup_progress_arg cpa; struct got_lonely_packidx_progress_arg lpa; off_t loose_before, loose_after; @@ -1306,6 +1307,10 @@ cmd_cleanup(int argc, char *argv[]) "this implies that objects must not be deleted"); goto done; } + + error = got_repo_cleanup_prepare(repo, &lock); + if (error) + goto done; if (remove_lonely_packidx) { memset(&lpa, 0, sizeof(lpa)); @@ -1381,6 +1386,7 @@ cmd_cleanup(int argc, char *argv[]) } done: + got_repo_cleanup_complete(repo, lock); if (repo) got_repo_close(repo); if (pack_fds) { blob - 57bb3f85a90ec6fdf48dc338be1a61ccab5a133c blob + 67ccceea225850c691fd8c0b8c3c4a6e397498dd --- include/got_repository_admin.h +++ include/got_repository_admin.h @@ -14,6 +14,8 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +struct got_lockfile; + /* A callback function which gets invoked with progress information to print. */ typedef const struct got_error *(*got_pack_progress_cb)(void *arg, int ncolored, int nfound, int ntrees, off_t packfile_size, int ncommits, @@ -68,6 +70,18 @@ got_repo_list_pack(FILE *packfile, struct got_object_i struct got_repository *repo, got_pack_list_cb list_cb, void *list_arg, got_cancel_cb cancel_cb, void *cancel_arg); +/* + * Prepare for removing loose objects or redundant packfiles. + * + * These functions do the necessary locking in order to avoid + * concurrent operation to irremediably damage the repository. + */ +const struct got_error * +got_repo_cleanup_prepare(struct got_repository *, struct got_lockfile **); + +const struct got_error * +got_repo_cleanup_complete(struct got_repository *, struct got_lockfile *); + /* A callback function which gets invoked with cleanup information to print. */ typedef const struct got_error *(*got_cleanup_progress_cb)(void *arg, int nloose, int ncommits, int npurged, int nredundant); blob - 8220c815a61bbe282585b0a67e7b39f74ad2a9a4 blob + f44259d099d9b70c547d238971a7cc147c4e209f --- lib/got_lib_repository.h +++ lib/got_lib_repository.h @@ -134,6 +134,9 @@ struct got_repository { /* Settings read from got.conf. */ struct got_gotconfig *gotconfig; + + /* cleanup lockfile */ + struct got_lockfile *cleanup_lock; }; const struct got_error*got_repo_cache_object(struct got_repository *, blob - e1a1870147bfefcb7e3caaaedf16d6b92e78a5c1 blob + e5276a2237daae080959cb6cfda569fa291cddbe --- lib/lockfile.c +++ lib/lockfile.c @@ -55,11 +55,11 @@ got_lockfile_lock(struct got_lockfile **lf, const char do { if (dir_fd != -1) { (*lf)->fd = openat(dir_fd, (*lf)->path, - O_RDONLY | O_CREAT | O_EXCL | O_EXLOCK | O_CLOEXEC, + O_RDWR | O_CREAT | O_EXCL | O_EXLOCK | O_CLOEXEC, GOT_DEFAULT_FILE_MODE); } else { (*lf)->fd = open((*lf)->path, - O_RDONLY | O_CREAT | O_EXCL | O_EXLOCK | O_CLOEXEC, + O_RDWR | O_CREAT | O_EXCL | O_EXLOCK | O_CLOEXEC, GOT_DEFAULT_FILE_MODE); } if ((*lf)->fd != -1) blob - 213bc1e83451612e1e2ba0be6bf664e3f4f44050 blob + bb314c3843f03978714c781199f72d807aeb3881 --- lib/repository_admin.c +++ lib/repository_admin.c @@ -611,6 +611,42 @@ done: if (packidx) got_packidx_close(packidx); return err; +} + +const struct got_error * +got_repo_cleanup_prepare(struct got_repository *repo, + struct got_lockfile **lk) +{ + const struct got_error *err; + char myname[HOST_NAME_MAX + 1]; + + if (gethostname(myname, sizeof(myname)) == -1) + return got_error_from_errno("gethostname"); + + err = got_lockfile_lock(lk, "gc.pid", got_repo_get_fd(repo)); + if (err) + return err; + + /* + * Git uses these info to provide some verbiage when finds a + * lock during `git gc --force' so don't try too hard to avoid + * short writes and don't care if a race happens between the + * lockfile creation and the write itself. + */ + if (dprintf((*lk)->fd, "%d %s", getpid(), myname) < 0) + return got_error_from_errno("dprintf"); + + return NULL; +} + +const struct got_error * +got_repo_cleanup_complete(struct got_repository *repo, + struct got_lockfile *lk) +{ + if (lk == NULL) + return NULL; + + return got_lockfile_unlock(lk, got_repo_get_fd(repo)); } static const struct got_error * @@ -1417,21 +1453,6 @@ got_repo_purge_redundant_packfiles(struct got_reposito if (idset) got_object_idset_free(idset); return err; -} - -static const struct got_error * -remove_packidx(int dir_fd, const char *relpath) -{ - const struct got_error *err, *unlock_err; - struct got_lockfile *lf; - - err = got_lockfile_lock(&lf, relpath, dir_fd); - if (err) - return err; - if (unlinkat(dir_fd, relpath, 0) == -1) - err = got_error_from_errno("unlinkat"); - unlock_err = got_lockfile_unlock(lf, dir_fd); - return err ? err : unlock_err; } const struct got_error * @@ -1491,9 +1512,10 @@ got_repo_remove_lonely_packidx(struct got_repository * } if (!dry_run) { - err = remove_packidx(packdir_fd, dent->d_name); - if (err) + if (unlinkat(packdir_fd, dent->d_name, 0) == -1) { + err = got_error_from_errno("unlinkat"); goto done; + } } if (progress_cb) { char *path;