[PATCH 3/4] refs.c: change update_refs to run the commit loops once all work is finished
- From
Ronnie Sahlberg <sahlberg@google.com>
- Date
- Apr 10, 2014, 18:30 UTC
- Message-ID
- <1397154625-11884-4-git-send-email-sahlberg@google.com>
- In-Reply-To
- <1397154625-11884-1-git-send-email-sahlberg@google.com>
During update_refs we will both be deleting refs as well as updating/changing the refs. Since both delete and update now use the same lock/update/commit pattern
lock = lock_ref_sha1_basic() (or varient of)
write_ref_sha1(lock)/delete_ref_loose(lock)
unlock_ref(lock) | commit_ref_lock(lock)we can now simplify update_update refs to have one loop that locks all involved refs. A second loop that writes or flags for deletion, but does not commit, all the refs. And a final third loop that commits all the refs once all the work and preparations are complete.
This makes updating/deleting multiple refs 'more atomic' since we will not start the commit phase until all the preparations have completed successfully.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> --- refs.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/refs.c b/refs.c index 038614d..1678e12 100644 --- a/refs.c +++ b/refs.c @@ -3368,34 +3368,38 @@ int update_refs(const char *action, const struct ref_update **updates_orig, } } - /* Perform updates first so live commits remain referenced */ - for (i = 0; i < n; i++) - if (!is_null_sha1(updates[i]->new_sha1)) { + /* Go through and prepare all updates and deletes */ + for (i = 0; i < n; i++) { + if (!is_null_sha1(updates[i]->new_sha1)) ret = update_ref_write(action, updates[i]->ref_name, updates[i]->new_sha1, locks[i], onerr); - if (ret) - unlock_ref(locks[i]); - else - commit_ref_lock(locks[i]); - locks[i] = NULL; - if (ret) - goto cleanup; + else { + delnames[delnum++] = locks[i]->ref_name; + ret = delete_ref_loose(locks[i], types[i]); } + if (ret) + goto cleanup; + } - /* Perform deletes now that updates are safely completed */ + ret = repack_without_refs(delnames, delnum); + for (i = 0; i < delnum; i++) + unlink_or_warn(git_path("logs/%s", delnames[i])); + clear_loose_ref_cache(&ref_cache); + + /* Perform updates first so live commits remain referenced */ + for (i = 0; i < n; i++) + if (locks[i] && !locks[i]->delete_ref) { + ret |= commit_ref_lock(locks[i]); + locks[i] = NULL; + } + /* And finally perform all deletes */ for (i = 0; i < n; i++) if (locks[i]) { - delnames[delnum++] = locks[i]->ref_name; - ret |= delete_ref_loose(locks[i], types[i]); ret |= commit_ref_lock(locks[i]); locks[i] = NULL; } - ret |= repack_without_refs(delnames, delnum); - for (i = 0; i < delnum; i++) - unlink_or_warn(git_path("logs/%s", delnames[i])); - clear_loose_ref_cache(&ref_cache); cleanup: for (i = 0; i < n; i++)
-- 1.9.1.478.ga5a8238.dirty