commit e81b604b157f332b391463c0d0b0a114981a5637 from: Omar Polo via: Thomas Adam date: Thu Jun 22 13:10:34 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 - d8963ff8248a747143b96569fa70c9c81104d63d commit + e81b604b157f332b391463c0d0b0a114981a5637 blob - 5be1d1bbffc9287159df30b4dc3e4420bda2539b blob + 05bbf129c161a7987a5dd3be1b38fa9ba12aa0a1 --- gotadmin/gotadmin.c +++ gotadmin/gotadmin.c @@ -1235,6 +1235,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; @@ -1305,6 +1306,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)); @@ -1380,6 +1385,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 - 7bad6c71de40a22d1c400051508cdc928fad6b7e blob + d793ba0ecff70de035bafaeda51106600264190c --- lib/lockfile.c +++ lib/lockfile.c @@ -57,11 +57,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 - 2187b12aa9b0522477825183261b30c78d674291 blob + dbfbfe97283f4b857964b519acf28b8bd6aef752 --- lib/repository_admin.c +++ lib/repository_admin.c @@ -608,6 +608,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 * @@ -1414,21 +1450,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 * @@ -1489,9 +1510,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;