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.
This commit is contained in:
Colin Walters 2021-12-03 14:35:12 -05:00
parent 267ca93da2
commit 2c39bd88a9
1 changed files with 2 additions and 8 deletions

View File

@ -6105,7 +6105,7 @@ summary_add_ref_entry (OstreeRepo *self,
* and refs in %OSTREE_SUMMARY_COLLECTION_MAP are guaranteed to be in * and refs in %OSTREE_SUMMARY_COLLECTION_MAP are guaranteed to be in
* lexicographic order. * lexicographic order.
* *
* Locking: exclusive * Locking: shared (Prior to 2021.7, this was exclusive)
*/ */
gboolean gboolean
ostree_repo_regenerate_summary (OstreeRepo *self, ostree_repo_regenerate_summary (OstreeRepo *self,
@ -6113,16 +6113,10 @@ ostree_repo_regenerate_summary (OstreeRepo *self,
GCancellable *cancellable, GCancellable *cancellable,
GError **error) 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; g_autoptr(OstreeRepoAutoLock) lock = NULL;
gboolean no_deltas_in_summary = FALSE; 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); cancellable, error);
if (!lock) if (!lock)
return FALSE; return FALSE;