packaging
/
ostree
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

lib/repo: Add MT support for transaction_set_ref(), clarify MT rules 

For rpm-ostree I'd like to do importing in parallel with threads; the code is
*almost* ready for that except today it calls
`ostree_repo_transaction_set_ref()`.

Looking at the code, there's really a "transaction" struct here,
not just stats.  Let's lift that struct out, and move the refs
into it under the existing lock.

Clarify the documentation around multithreading for various functions.

Closes: #1358
Approved by: jlebon
This commit is contained in:
Colin Walters 2017-12-01 15:18:37 -05:00 committed by Atomic Bot
parent 8d6b30c381
commit 89a57bb6d8
3 changed files with 108 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

151
src/libostree/ostree-repo-commit.c
View File

@ -658,19 +658,19 @@ write_content_object (OstreeRepo *self,
/* Free space check; only applies during transactions */ /* Free space check; only applies during transactions */
if (self->min_free_space_percent > 0 && self->in_transaction) if (self->min_free_space_percent > 0 && self->in_transaction)
{ {
g_mutex_lock (&self->txn_stats_lock); g_mutex_lock (&self->txn_lock);
g_assert_cmpint (self->txn_blocksize, >, 0); g_assert_cmpint (self->txn.blocksize, >, 0);
const fsblkcnt_t object_blocks = (size / self->txn_blocksize) + 1; const fsblkcnt_t object_blocks = (size / self->txn.blocksize) + 1;
if (object_blocks > self->max_txn_blocks) if (object_blocks > self->txn.max_blocks)
{ {
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
g_autofree char *formatted_required = g_format_size ((guint64)object_blocks * self->txn_blocksize); g_autofree char *formatted_required = g_format_size ((guint64)object_blocks * self->txn.blocksize);
return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required", return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required",
self->min_free_space_percent, formatted_required); self->min_free_space_percent, formatted_required);
} }
/* This is the main bit that needs mutex protection */ /* This is the main bit that needs mutex protection */
self->max_txn_blocks -= object_blocks; self->txn.max_blocks -= object_blocks;
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
} }
/* For regular files, we create them with default mode, and only /* For regular files, we create them with default mode, and only
@ -777,9 +777,9 @@ write_content_object (OstreeRepo *self,
/* If we already have it, just update the stats. */ /* If we already have it, just update the stats. */
if (have_obj) if (have_obj)
{ {
g_mutex_lock (&self->txn_stats_lock); g_mutex_lock (&self->txn_lock);
self->txn_stats.content_objects_total++; self->txn.stats.content_objects_total++;
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
if (out_csum) if (out_csum)
*out_csum = ostree_checksum_to_bytes (actual_checksum); *out_csum = ostree_checksum_to_bytes (actual_checksum);
/* Note early return */ /* Note early return */
@ -849,11 +849,11 @@ write_content_object (OstreeRepo *self,
} }
/* Update statistics */ /* Update statistics */
g_mutex_lock (&self->txn_stats_lock); g_mutex_lock (&self->txn_lock);
self->txn_stats.content_objects_written++; self->txn.stats.content_objects_written++;
self->txn_stats.content_bytes_written += file_object_length; self->txn.stats.content_bytes_written += file_object_length;
self->txn_stats.content_objects_total++; self->txn.stats.content_objects_total++;
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
if (out_csum) if (out_csum)
{ {
@ -1018,9 +1018,9 @@ write_metadata_object (OstreeRepo *self,
*/ */
if (have_obj) if (have_obj)
{ {
g_mutex_lock (&self->txn_stats_lock); g_mutex_lock (&self->txn_lock);
self->txn_stats.metadata_objects_total++; self->txn.stats.metadata_objects_total++;
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
if (out_csum) if (out_csum)
*out_csum = ostree_checksum_to_bytes (actual_checksum); *out_csum = ostree_checksum_to_bytes (actual_checksum);
@ -1090,10 +1090,10 @@ write_metadata_object (OstreeRepo *self,
} }
/* Update the stats, note we both wrote one and add to total */ /* Update the stats, note we both wrote one and add to total */
g_mutex_lock (&self->txn_stats_lock); g_mutex_lock (&self->txn_lock);
self->txn_stats.metadata_objects_written++; self->txn.stats.metadata_objects_written++;
self->txn_stats.metadata_objects_total++; self->txn.stats.metadata_objects_total++;
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
if (out_csum) if (out_csum)
*out_csum = ostree_checksum_to_bytes (actual_checksum); *out_csum = ostree_checksum_to_bytes (actual_checksum);
@ -1259,6 +1259,8 @@ devino_cache_lookup (OstreeRepo *self,
* existing ostree objects, then this will speed up considerably, so call it * existing ostree objects, then this will speed up considerably, so call it
* before you call ostree_write_directory_to_mtree() or similar. However, * before you call ostree_write_directory_to_mtree() or similar. However,
* ostree_repo_devino_cache_new() is better as it avoids scanning all objects. * ostree_repo_devino_cache_new() is better as it avoids scanning all objects.
*
* Multithreading: This function is *not* MT safe.
*/ */
gboolean gboolean
ostree_repo_scan_hardlinks (OstreeRepo *self, ostree_repo_scan_hardlinks (OstreeRepo *self,
@ -1287,8 +1289,17 @@ ostree_repo_scan_hardlinks (OstreeRepo *self,
* ostree_repo_commit_transaction(), or abort the transaction with * ostree_repo_commit_transaction(), or abort the transaction with
* ostree_repo_abort_transaction(). * ostree_repo_abort_transaction().
* *
* Currently, transactions are not atomic, and aborting a transaction * Currently, transactions may result in partial commits or data in the target
* will not erase any data you write during the transaction. * repository if interrupted during ostree_repo_commit_transaction(), and
* further writing refs is also not currently atomic.
*
* There can be at most one transaction active on a repo at a time per instance
* of `OstreeRepo`; however, it is safe to have multiple threads writing objects
* on a single `OstreeRepo` instance as long as their lifetime is bounded by the
* transaction.
*
* Multithreading: This function is *not* MT safe; only one transaction can be
* active at a time.
*/ */
gboolean gboolean
ostree_repo_prepare_transaction (OstreeRepo *self, ostree_repo_prepare_transaction (OstreeRepo *self,
@ -1299,7 +1310,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self,
g_return_val_if_fail (self->in_transaction == FALSE, FALSE); g_return_val_if_fail (self->in_transaction == FALSE, FALSE);
memset (&self->txn_stats, 0, sizeof (OstreeRepoTransactionStats)); memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats));
self->in_transaction = TRUE; self->in_transaction = TRUE;
if (self->min_free_space_percent > 0) if (self->min_free_space_percent > 0)
@ -1307,23 +1318,23 @@ ostree_repo_prepare_transaction (OstreeRepo *self,
struct statvfs stvfsbuf; struct statvfs stvfsbuf;
if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0)
return glnx_throw_errno_prefix (error, "fstatvfs"); return glnx_throw_errno_prefix (error, "fstatvfs");
g_mutex_lock (&self->txn_stats_lock); g_mutex_lock (&self->txn_lock);
self->txn_blocksize = stvfsbuf.f_bsize; self->txn.blocksize = stvfsbuf.f_bsize;
/* Convert fragment to blocks to compute the total */ /* Convert fragment to blocks to compute the total */
guint64 total_blocks = (stvfsbuf.f_frsize * stvfsbuf.f_blocks) / stvfsbuf.f_bsize; guint64 total_blocks = (stvfsbuf.f_frsize * stvfsbuf.f_blocks) / stvfsbuf.f_bsize;
/* Use the appropriate free block count if we're unprivileged */ /* Use the appropriate free block count if we're unprivileged */
guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree);
guint64 reserved_blocks = ((double)total_blocks) * (self->min_free_space_percent/100.0); guint64 reserved_blocks = ((double)total_blocks) * (self->min_free_space_percent/100.0);
if (bfree > reserved_blocks) if (bfree > reserved_blocks)
self->max_txn_blocks = bfree - reserved_blocks; self->txn.max_blocks = bfree - reserved_blocks;
else else
{ {
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
g_autofree char *formatted_free = g_format_size (bfree * self->txn_blocksize); g_autofree char *formatted_free = g_format_size (bfree * self->txn.blocksize);
return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available",
self->min_free_space_percent, formatted_free); self->min_free_space_percent, formatted_free);
} }
g_mutex_unlock (&self->txn_stats_lock); g_mutex_unlock (&self->txn_lock);
} }
gboolean ret_transaction_resume = FALSE; gboolean ret_transaction_resume = FALSE;
@ -1547,10 +1558,10 @@ cleanup_tmpdir (OstreeRepo *self,
static void static void
ensure_txn_refs (OstreeRepo *self) ensure_txn_refs (OstreeRepo *self)
{ {
if (self->txn_refs == NULL) if (self->txn.refs == NULL)
self->txn_refs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); self->txn.refs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
if (self->txn_collection_refs == NULL) if (self->txn.collection_refs == NULL)
self->txn_collection_refs = g_hash_table_new_full (ostree_collection_ref_hash, self->txn.collection_refs = g_hash_table_new_full (ostree_collection_ref_hash,
ostree_collection_ref_equal, ostree_collection_ref_equal,
(GDestroyNotify) ostree_collection_ref_free, (GDestroyNotify) ostree_collection_ref_free,
g_free); g_free);
@ -1565,6 +1576,8 @@ ensure_txn_refs (OstreeRepo *self)
* Like ostree_repo_transaction_set_ref(), but takes concatenated * Like ostree_repo_transaction_set_ref(), but takes concatenated
* @refspec format as input instead of separate remote and name * @refspec format as input instead of separate remote and name
* arguments. * arguments.
*
* Multithreading: Since v2017.15 this function is MT safe.
*/ */
void void
ostree_repo_transaction_set_refspec (OstreeRepo *self, ostree_repo_transaction_set_refspec (OstreeRepo *self,
@ -1573,9 +1586,10 @@ ostree_repo_transaction_set_refspec (OstreeRepo *self,
{ {
g_return_if_fail (self->in_transaction == TRUE); g_return_if_fail (self->in_transaction == TRUE);
g_mutex_lock (&self->txn_lock);
ensure_txn_refs (self); ensure_txn_refs (self);
g_hash_table_replace (self->txn.refs, g_strdup (refspec), g_strdup (checksum));
g_hash_table_replace (self->txn_refs, g_strdup (refspec), g_strdup (checksum)); g_mutex_unlock (&self->txn_lock);
} }
/** /**
@ -1592,10 +1606,20 @@ ostree_repo_transaction_set_refspec (OstreeRepo *self,
* Otherwise, if @checksum is %NULL, then record that the ref should * Otherwise, if @checksum is %NULL, then record that the ref should
* be deleted. * be deleted.
* *
* The change will not be written out immediately, but when the transaction * The change will be written when the transaction is completed with
* is completed with ostree_repo_commit_transaction(). If the transaction * ostree_repo_commit_transaction(); that function takes care of writing all of
* is instead aborted with ostree_repo_abort_transaction(), no changes will * the objects (such as the commit referred to by @checksum) before updating the
* be made to the repository. * refs. If the transaction is instead aborted with
* ostree_repo_abort_transaction(), no changes to the ref will be made to the
* repository.
*
* Note however that currently writing *multiple* refs is not truly atomic; if
* the process or system is terminated during
* ostree_repo_commit_transaction(), it is possible that just some of the refs
* will have been updated. Your application should take care to handle this
* case.
*
* Multithreading: Since v2017.15 this function is MT safe.
*/ */
void void
ostree_repo_transaction_set_ref (OstreeRepo *self, ostree_repo_transaction_set_ref (OstreeRepo *self,
@ -1603,18 +1627,18 @@ ostree_repo_transaction_set_ref (OstreeRepo *self,
const char *ref, const char *ref,
const char *checksum) const char *checksum)
{ {
char *refspec;
g_return_if_fail (self->in_transaction == TRUE); g_return_if_fail (self->in_transaction == TRUE);
ensure_txn_refs (self); char *refspec;
if (remote) if (remote)
refspec = g_strdup_printf ("%s:%s", remote, ref); refspec = g_strdup_printf ("%s:%s", remote, ref);
else else
refspec = g_strdup (ref); refspec = g_strdup (ref);
g_hash_table_replace (self->txn_refs, refspec, g_strdup (checksum)); g_mutex_lock (&self->txn_lock);
ensure_txn_refs (self);
g_hash_table_replace (self->txn.refs, refspec, g_strdup (checksum));
g_mutex_unlock (&self->txn_lock);
} }
/** /**
@ -1634,6 +1658,8 @@ ostree_repo_transaction_set_ref (OstreeRepo *self,
* is instead aborted with ostree_repo_abort_transaction(), no changes will * is instead aborted with ostree_repo_abort_transaction(), no changes will
* be made to the repository. * be made to the repository.
* *
* Multithreading: Since v2017.15 this function is MT safe.
*
* Since: 2017.8 * Since: 2017.8
*/ */
void void
@ -1646,10 +1672,11 @@ ostree_repo_transaction_set_collection_ref (OstreeRepo *self,
g_return_if_fail (ref != NULL); g_return_if_fail (ref != NULL);
g_return_if_fail (checksum == NULL || ostree_validate_checksum_string (checksum, NULL)); g_return_if_fail (checksum == NULL || ostree_validate_checksum_string (checksum, NULL));
g_mutex_lock (&self->txn_lock);
ensure_txn_refs (self); ensure_txn_refs (self);
g_hash_table_replace (self->txn.collection_refs,
g_hash_table_replace (self->txn_collection_refs,
ostree_collection_ref_dup (ref), g_strdup (checksum)); ostree_collection_ref_dup (ref), g_strdup (checksum));
g_mutex_unlock (&self->txn_lock);
} }
/** /**
@ -1664,6 +1691,8 @@ ostree_repo_transaction_set_collection_ref (OstreeRepo *self,
* This is like ostree_repo_transaction_set_ref(), except it may be * This is like ostree_repo_transaction_set_ref(), except it may be
* invoked outside of a transaction. This is presently safe for the * invoked outside of a transaction. This is presently safe for the
* case where we're creating or overwriting an existing ref. * case where we're creating or overwriting an existing ref.
*
* Multithreading: This function is MT safe.
*/ */
gboolean gboolean
ostree_repo_set_ref_immediate (OstreeRepo *self, ostree_repo_set_ref_immediate (OstreeRepo *self,
@ -1745,6 +1774,12 @@ ostree_repo_set_collection_ref_immediate (OstreeRepo *self,
* Complete the transaction. Any refs set with * Complete the transaction. Any refs set with
* ostree_repo_transaction_set_ref() or * ostree_repo_transaction_set_ref() or
* ostree_repo_transaction_set_refspec() will be written out. * ostree_repo_transaction_set_refspec() will be written out.
*
* Note that if multiple threads are performing writes, all such threads must
* have terminated before this function is invoked.
*
* Multithreading: This function is *not* MT safe; only one transaction can be
* active at a time.
*/ */
gboolean gboolean
ostree_repo_commit_transaction (OstreeRepo *self, ostree_repo_commit_transaction (OstreeRepo *self,
@ -1782,15 +1817,15 @@ ostree_repo_commit_transaction (OstreeRepo *self,
if (self->loose_object_devino_hash) if (self->loose_object_devino_hash)
g_hash_table_remove_all (self->loose_object_devino_hash); g_hash_table_remove_all (self->loose_object_devino_hash);
if (self->txn_refs) if (self->txn.refs)
if (!_ostree_repo_update_refs (self, self->txn_refs, cancellable, error)) if (!_ostree_repo_update_refs (self, self->txn.refs, cancellable, error))
return FALSE; return FALSE;
g_clear_pointer (&self->txn_refs, g_hash_table_destroy); g_clear_pointer (&self->txn.refs, g_hash_table_destroy);
if (self->txn_collection_refs) if (self->txn.collection_refs)
if (!_ostree_repo_update_collection_refs (self, self->txn_collection_refs, cancellable, error)) if (!_ostree_repo_update_collection_refs (self, self->txn.collection_refs, cancellable, error))
return FALSE; return FALSE;
g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy);
self->in_transaction = FALSE; self->in_transaction = FALSE;
@ -1798,7 +1833,7 @@ ostree_repo_commit_transaction (OstreeRepo *self,
return FALSE; return FALSE;
if (out_stats) if (out_stats)
*out_stats = self->txn_stats; *out_stats = self->txn.stats;
return TRUE; return TRUE;
} }
@ -1829,8 +1864,8 @@ ostree_repo_abort_transaction (OstreeRepo *self,
if (self->loose_object_devino_hash) if (self->loose_object_devino_hash)
g_hash_table_remove_all (self->loose_object_devino_hash); g_hash_table_remove_all (self->loose_object_devino_hash);
g_clear_pointer (&self->txn_refs, g_hash_table_destroy); g_clear_pointer (&self->txn.refs, g_hash_table_destroy);
g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy);
glnx_tmpdir_unset (&self->commit_stagedir); glnx_tmpdir_unset (&self->commit_stagedir);
glnx_release_lock_file (&self->commit_stagedir_lock); glnx_release_lock_file (&self->commit_stagedir_lock);

18
src/libostree/ostree-repo-private.h
View File

@ -82,6 +82,15 @@ typedef enum {
OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE, /* We match /ostree/repo */ OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE, /* We match /ostree/repo */
} OstreeRepoSysrootKind; } OstreeRepoSysrootKind;
typedef struct {
GHashTable *refs; /* (element-type utf8 utf8) */
GHashTable *collection_refs; /* (element-type OstreeCollectionRef utf8) */
OstreeRepoTransactionStats stats;
/* Implementation of min-free-space-percent */
gulong blocksize;
fsblkcnt_t max_blocks;
} OstreeRepoTxn;
/** /**
* OstreeRepo: * OstreeRepo:
* *
@ -109,13 +118,8 @@ struct OstreeRepo {
GWeakRef sysroot; /* Weak to avoid a circular ref; see also `is_system` */ GWeakRef sysroot; /* Weak to avoid a circular ref; see also `is_system` */
char *remotes_config_dir; char *remotes_config_dir;
GHashTable *txn_refs; /* (element-type utf8 utf8) */ GMutex txn_lock;
GHashTable *txn_collection_refs; /* (element-type OstreeCollectionRef utf8) */ OstreeRepoTxn txn;
GMutex txn_stats_lock;
OstreeRepoTransactionStats txn_stats;
/* Implementation of min-free-space-percent */
gulong txn_blocksize;
fsblkcnt_t max_txn_blocks;
GMutex cache_lock; GMutex cache_lock;
guint dirmeta_cache_refcount; guint dirmeta_cache_refcount;

8
src/libostree/ostree-repo.c
View File

@ -505,13 +505,13 @@ ostree_repo_finalize (GObject *object)
g_hash_table_destroy (self->updated_uncompressed_dirs); g_hash_table_destroy (self->updated_uncompressed_dirs);
if (self->config) if (self->config)
g_key_file_free (self->config); g_key_file_free (self->config);
g_clear_pointer (&self->txn_refs, g_hash_table_destroy); g_clear_pointer (&self->txn.refs, g_hash_table_destroy);
g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy);
g_clear_error (&self->writable_error); g_clear_error (&self->writable_error);
g_clear_pointer (&self->object_sizes, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&self->object_sizes, (GDestroyNotify) g_hash_table_unref);
g_clear_pointer (&self->dirmeta_cache, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&self->dirmeta_cache, (GDestroyNotify) g_hash_table_unref);
g_mutex_clear (&self->cache_lock); g_mutex_clear (&self->cache_lock);
g_mutex_clear (&self->txn_stats_lock); g_mutex_clear (&self->txn_lock);
g_free (self->collection_id); g_free (self->collection_id);
g_clear_pointer (&self->remotes, g_hash_table_destroy); g_clear_pointer (&self->remotes, g_hash_table_destroy);
@ -672,7 +672,7 @@ ostree_repo_init (OstreeRepo *self)
test_error_keys, G_N_ELEMENTS (test_error_keys)); test_error_keys, G_N_ELEMENTS (test_error_keys));
g_mutex_init (&self->cache_lock); g_mutex_init (&self->cache_lock);
g_mutex_init (&self->txn_stats_lock); g_mutex_init (&self->txn_lock);
self->remotes = g_hash_table_new_full (g_str_hash, g_str_equal, self->remotes = g_hash_table_new_full (g_str_hash, g_str_equal,
(GDestroyNotify) NULL, (GDestroyNotify) NULL,