73 lines
2.9 KiB
Diff
73 lines
2.9 KiB
Diff
From 2c39bd88a9b0db207c1158c76bbbe0b484dec9dd Mon Sep 17 00:00:00 2001
|
|
From: Colin Walters <walters@verbum.org>
|
|
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 74cea37f2..a9be9f948 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;
|
|
|