git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/3] Change update_refs to run a single commit loop once all work is finished.

From
Ronnie Sahlberg <sahlberg@google.com>
Date
Apr 8, 2014, 23:21 UTC
Message-ID
<1396999284-4691-4-git-send-email-sahlberg@google.com>
In-Reply-To
<1396999284-4691-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 | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/refs.c b/refs.c
index 038614d..e94099b 100644
--- a/refs.c
+++ b/refs.c
@@ -3369,33 +3369,31 @@ 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)) {
+	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);
+
+	/* Commit all the updates/deletions */
 	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.477.g7668a0d.dirty
Previous: Ronnie SahlbergNext: Junio C Hamano
Message 4 of 5 in “Make update-refs more atomic”
  1. 0/3 Make update-refs more atomicRonnie Sahlberg, Apr 8, 2014
  2. 1/3 Split writing and commiting a ref into two separate functions.Ronnie Sahlberg, Apr 8, 2014
  3. 2/3 Split delete_ref_loose() into a separate flag-for-deletion and commit phaseRonnie Sahlberg, Apr 8, 2014
  4. 3/3 Change update_refs to run a single commit loop once all work is finished.Ronnie Sahlberg, Apr 8, 2014
  5. Junio C HamanoApr 9, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.