repo: Change locking for summary regeneration to be shared
authorColin Walters <walters@verbum.org>
Fri, 3 Dec 2021 19:35:12 +0000 (14:35 -0500)
committerColin Walters <walters@verbum.org>
Fri, 3 Dec 2021 19:42:03 +0000 (14:42 -0500)
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

index 74cea37f235ff0712c1faf7e4335bcff5020040e..a9be9f948e1ec1be718e972f211c18f26e5b70c0 100644 (file)
@@ -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;