From 2c39bd88a9b0db207c1158c76bbbe0b484dec9dd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 3 Dec 2021 14:35:12 -0500 Subject: [PATCH] repo: Change locking for summary regeneration to be shared This is trying to address: https://pagure.io/fedora-iot/issue/48 Basically we changed rpm-ostree to start doing a shared lock during commit by default, but this broke because pungi is starting a process doing a commit for each architecture, and then trying to regenerate the summary after each one. This patch is deleting a big comment with a rationale for why summary regeneration should be exclusive. Point by point: > This makes sure the commits and deltas don't get > deleted while generating the summary. But prune operations require an exclusive lock, which means that data still can't be deleted when the summary grabs a shared lock. > It also means we can be sure refs > won't be created/updated/deleted during the operation, without having to > add exclusive locks to those operations which would prevent concurrent > commits from working. First: The status quo *has* prevented concurrent commits from working! There is no real locking solution to this problem. What we really need to do here is regenerate the summary after each commit *or* when the caller decides to do it and e.g. include deltas at the same time. It's OK if multiple threads race to regenerate the summary; last-one-wins behavior here is totally fine. --- src/libostree/ostree-repo.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 74cea37f23..a9be9f948e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -6105,7 +6105,7 @@ summary_add_ref_entry (OstreeRepo *self, * and refs in %OSTREE_SUMMARY_COLLECTION_MAP are guaranteed to be in * lexicographic order. * - * Locking: exclusive + * Locking: shared (Prior to 2021.7, this was exclusive) */ gboolean ostree_repo_regenerate_summary (OstreeRepo *self, @@ -6113,16 +6113,10 @@ ostree_repo_regenerate_summary (OstreeRepo *self, GCancellable *cancellable, GError **error) { - /* Take an exclusive lock. This makes sure the commits and deltas don't get - * deleted while generating the summary. It also means we can be sure refs - * won't be created/updated/deleted during the operation, without having to - * add exclusive locks to those operations which would prevent concurrent - * commits from working. - */ g_autoptr(OstreeRepoAutoLock) lock = NULL; gboolean no_deltas_in_summary = FALSE; - lock = ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_EXCLUSIVE, + lock = ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_SHARED, cancellable, error); if (!lock) return FALSE;