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

[PATCH v3 03/16] refs.c: use packed refs when deleting refs during a transaction

From
Ronnie Sahlberg <sahlberg@google.com>
Date
Nov 7, 2014, 19:38 UTC
Message-ID
<1415389145-6391-4-git-send-email-sahlberg@google.com>
In-Reply-To
<1415389145-6391-1-git-send-email-sahlberg@google.com>

Make the deletion of refs during a transaction more atomic. Start by first copying all loose refs we will be deleting to the packed refs file and then commit the packed refs file. Then re-lock the packed refs file to stop anyone else from modifying these refs and keep it locked until we are finished. Since all refs we are about to delete are now safely held in the packed refs file we can proceed to immediately unlink any corresponding loose refs and still be fully rollback-able.

The exception is for refs that can not be resolved. Those refs are never added to the packed refs and will just be un-rollback-ably deleted during commit.

By deleting all the loose refs at the start of the transaction we make make it possible to both delete one ref and then re-create a different ref in the same transaction even if the two refs would normally conflict.

Example: rename m->m/m

In that example we want to delete the file 'm' so that we make room so that we can create a directory with the same name in order to lock and write to the ref m/m and its lock-file m/m.lock.

If there is a failure during the commit phase we can rollback without losing any refs since we have so far only deleted loose refs that that are guaranteed to also have a corresponding entry in the packed refs file. Once we have finished all updates for refs and their reflogs we can repack the packed refs file and remove the to-be-deleted refs from the packed refs, at which point all the deleted refs will disappear in one atomic rename operation.

This also means that for an outside observer, deletion of multiple refs in a single transaction will look atomic instead of one ref being deleted at a time.

In order to do all this we need to change the semantics for the repack_without_refs function so that we can lock the packed refs file, do other stuff, and later be able to call repack_without_refs with the lock already taken. This means we need some additional changes in remote.c to reflect the changes to the repack_without_refs semantics.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/remote.c |  20 ++++++-
 refs.c           | 155 ++++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 138 insertions(+), 37 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 7f28f92..c25420f 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "lockfile.h"
 #include "parse-options.h"
 #include "transport.h"
 #include "remote.h"
@@ -753,6 +754,15 @@ static int remove_branches(struct string_list *branches)
 	const char **branch_names;
 	int i, result = 0;
 
+	if (lock_packed_refs(0)) {
+		struct strbuf err = STRBUF_INIT;
+
+		unable_to_lock_message(git_path("packed-refs"), errno, &err);
+		error("%s", err.buf);
+		strbuf_release(&err);
+		return -1;
+	}
+
 	branch_names = xmalloc(branches->nr * sizeof(*branch_names));
 	for (i = 0; i < branches->nr; i++)
 		branch_names[i] = branches->items[i].string;
@@ -1337,9 +1347,15 @@ static int prune_remote(const char *remote, int dry_run)
 			delete_refs[i] = states.stale.items[i].util;
 		if (!dry_run) {
 			struct strbuf err = STRBUF_INIT;
-			if (repack_without_refs(delete_refs, states.stale.nr,
-						&err))
+
+			if (lock_packed_refs(0)) {
+				unable_to_lock_message(git_path("packed-refs"),
+						       errno, &err);
 				result |= error("%s", err.buf);
+			} else
+				if (repack_without_refs(delete_refs,
+							states.stale.nr, &err))
+					result |= error("%s", err.buf);
 			strbuf_release(&err);
 		}
 		free(delete_refs);
diff --git a/refs.c b/refs.c
index d4ab65c..809bd3f 100644
--- a/refs.c
+++ b/refs.c
@@ -2660,6 +2660,9 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
 	return 0;
 }
 
+/*
+ * Must be called with packed refs already locked (and sorted)
+ */
 int repack_without_refs(const char **refnames, int n, struct strbuf *err)
 {
 	struct ref_dir *packed;
@@ -2674,14 +2677,12 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
 		if (get_packed_ref(refnames[i]))
 			break;
 
-	/* Avoid locking if we have nothing to do */
-	if (i == n)
+	/* Avoid processing if we have nothing to do */
+	if (i == n) {
+		rollback_packed_refs();
 		return 0; /* no refname exists in packed refs */
-
-	if (lock_packed_refs(0)) {
-		unable_to_lock_message(git_path("packed-refs"), errno, err);
-		return -1;
 	}
+
 	packed = get_packed_refs(&ref_cache);
 
 	/* Remove refnames from the cache */
@@ -3798,10 +3799,12 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,
 int transaction_commit(struct transaction *transaction,
 		       struct strbuf *err)
 {
-	int ret = 0, delnum = 0, i;
+	int ret = 0, delnum = 0, i, need_repack = 0;
 	const char **delnames;
 	int n = transaction->nr;
+	struct packed_ref_cache *packed_ref_cache;
 	struct ref_update **updates = transaction->updates;
+	struct ref_dir *packed;
 
 	assert(err);
 
@@ -3823,23 +3826,109 @@ int transaction_commit(struct transaction *transaction,
 		goto cleanup;
 	}
 
-	/* Acquire all ref locks while verifying old values */
+	/* Lock packed refs during commit */
+	if (lock_packed_refs(0)) {
+		if (err)
+			unable_to_lock_message(git_path("packed-refs"),
+					       errno, err);
+		ret = -1;
+		goto cleanup;
+	}
+
+	/* any loose refs are to be deleted are first copied to packed refs */
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
-		int flags = update->flags;
+		unsigned char sha1[20];
 
 		if (update->update_type != UPDATE_SHA1)
 			continue;
+		if (!is_null_sha1(update->new_sha1))
+			continue;
+		if (get_packed_ref(update->refname))
+			continue;
+		if (!resolve_ref_unsafe(update->refname, RESOLVE_REF_READING,
+					sha1, NULL))
+			continue;
 
-		if (is_null_sha1(update->new_sha1))
-			flags |= REF_DELETING;
+		add_packed_ref(update->refname, sha1);
+		need_repack = 1;
+	}
+	if (need_repack) {
+		packed = get_packed_refs(&ref_cache);;
+		sort_ref_dir(packed);
+		if (commit_packed_refs()){
+			strbuf_addf(err, "unable to overwrite old ref-pack "
+				    "file");
+			ret = -1;
+			goto cleanup;
+		}
+		/* lock the packed refs again so no one can change it */
+		if (lock_packed_refs(0)) {
+			if (err)
+				unable_to_lock_message(git_path("packed-refs"),
+						       errno, err);
+			ret = -1;
+			goto cleanup;
+		}
+	}
+
+	/*
+	 * At this stage any refs that are to be deleted have been moved to the
+	 * packed refs file anf the packed refs file is deleted. We can now
+	 * safely delete these loose refs.
+	 */
+
+	/* Unlink any loose refs scheduled for deletion */
+	for (i = 0; i < n; i++) {
+		struct ref_update *update = updates[i];
+
+		if (update->update_type != UPDATE_SHA1)
+			continue;
+		if (!is_null_sha1(update->new_sha1))
+			continue;
+		update->flags |= REF_DELETING;
+		update->lock = lock_ref_sha1_basic(update->refname,
+						   (update->have_old ?
+						    update->old_sha1 :
+						    NULL),
+						   NULL,
+						   update->flags,
+						   &update->type);
+		if (!update->lock) {
+			int df_conflict = (errno == ENOTDIR);
+
+			strbuf_addf(err, "Cannot lock the ref '%s'.",
+				    update->refname);
+			ret = df_conflict ?
+			  TRANSACTION_NAME_CONFLICT : 
+			  TRANSACTION_GENERIC_ERROR;
+			goto cleanup;
+		}
+		if (delete_ref_loose(update->lock, update->type, err)) {
+			ret = -1;
+			goto cleanup;
+		}
+		try_remove_empty_parents((char *)update->refname);
+		if (!(update->flags & REF_ISPRUNING))
+			  delnames[delnum++] = xstrdup(update->lock->ref_name);
+		unlock_ref(update->lock);
+		update->lock = NULL;
+	}
+
+	/* Acquire all ref locks for updates while verifying old values */
+	for (i = 0; i < n; i++) {
+		struct ref_update *update = updates[i];
 
+		if (update->update_type != UPDATE_SHA1)
+			continue;
+		if (is_null_sha1(update->new_sha1))
+			continue;
 		update->lock = lock_ref_sha1_basic(update->refname,
 						   (update->have_old ?
 						    update->old_sha1 :
 						    NULL),
 						   NULL,
-						   flags,
+						   update->flags,
 						   &update->type);
 		if (!update->lock) {
 			ret = (errno == ENOTDIR)
@@ -3851,6 +3940,10 @@ int transaction_commit(struct transaction *transaction,
 		}
 	}
 
+	/* delete reflog for all deleted refs */
+	for (i = 0; i < delnum; i++)
+		unlink_or_warn(git_path("logs/%s", delnames[i]));
+
 	/* Lock all reflog files */
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
@@ -3862,6 +3955,16 @@ int transaction_commit(struct transaction *transaction,
 			update->reflog_lock = update->orig_update->reflog_lock;
 			continue;
 		}
+		if (log_all_ref_updates && !reflog_exists(update->refname) &&
+		    create_reflog(update->refname)) {
+			ret = -1;
+			if (err)
+				strbuf_addf(err, "Failed to setup reflog for "
+					    "%s", update->refname);
+			goto cleanup;
+		}
+		if (!reflog_exists(update->refname))
+			continue;
 		update->reflog_fd = hold_lock_file_for_append(
 					update->reflog_lock,
 					git_path("logs/%s", update->refname),
@@ -3877,7 +3980,7 @@ int transaction_commit(struct transaction *transaction,
 		}
 	}
 
-	/* Perform ref updates first so live commits remain referenced */
+	/* Perform ref updates */
 	for (i = 0; i < n; i++) {
 		struct ref_update *update = updates[i];
 
@@ -3896,23 +3999,6 @@ int transaction_commit(struct transaction *transaction,
 		}
 	}
 
-	/* Perform deletes now that updates are safely completed */
-	for (i = 0; i < n; i++) {
-		struct ref_update *update = updates[i];
-
-		if (update->update_type != UPDATE_SHA1)
-			continue;
-		if (update->lock) {
-			if (delete_ref_loose(update->lock, update->type, err)) {
-				ret = TRANSACTION_GENERIC_ERROR;
-				goto cleanup;
-			}
-
-			if (!(update->flags & REF_ISPRUNING))
-				delnames[delnum++] = update->lock->ref_name;
-		}
-	}
-
 	/*
 	 * Update all reflog files
 	 * We have already committed all ref updates and deletes.
@@ -3963,15 +4049,14 @@ int transaction_commit(struct transaction *transaction,
 		}
 	}
 
-	if (repack_without_refs(delnames, delnum, err)) {
+	if (repack_without_refs(delnames, delnum, err))
 		ret = TRANSACTION_GENERIC_ERROR;
-		goto cleanup;
-	}
-	for (i = 0; i < delnum; i++)
-		unlink_or_warn(git_path("logs/%s", delnames[i]));
 	clear_loose_ref_cache(&ref_cache);
 
 cleanup:
+	packed_ref_cache = get_packed_ref_cache(&ref_cache);
+	if (packed_ref_cache->lock)
+		rollback_packed_refs();
 	transaction->state = TRANSACTION_CLOSED;
 
 	for (i = 0; i < n; i++)

-- 
2.1.0.rc2.206.gedb03e5
Previous: Ronnie SahlbergNext: Ronnie Sahlberg
Message 4 of 17 in “ref-transaction-rename”
  1. 00/16 ref-transaction-renameRonnie Sahlberg, Nov 7, 2014
  2. 01/16 refs.c: allow passing raw git_committer_info as email to _update_reflogRonnie Sahlberg, Nov 7, 2014
  3. 02/16 refs.c: return error instead of dying when locking fails during transactionRonnie Sahlberg, Nov 7, 2014
  4. 03/16 refs.c: use packed refs when deleting refs during a transactionRonnie Sahlberg, Nov 7, 2014
  5. 04/16 refs.c: use a stringlist for repack_without_refsRonnie Sahlberg, Nov 7, 2014
  6. 05/16 refs.c: add transaction support for renaming a reflogRonnie Sahlberg, Nov 7, 2014
  7. 06/16 refs.c: update rename_ref to use a transactionRonnie Sahlberg, Nov 7, 2014
  8. 07/16 refs.c: rollback the lockfile before we die() in repack_without_refsRonnie Sahlberg, Nov 7, 2014
  9. 08/16 refs.c: move reflog updates into its own functionRonnie Sahlberg, Nov 7, 2014
  10. 09/16 refs.c: write updates to packed refs when a transaction has more than one refRonnie Sahlberg, Nov 7, 2014
  11. 10/16 remote.c: use a transaction for deleting refsRonnie Sahlberg, Nov 7, 2014
  12. 11/16 refs.c: make repack_without_refs staticRonnie Sahlberg, Nov 7, 2014
  13. 12/16 refs.c: make the *_packed_refs functions staticRonnie Sahlberg, Nov 7, 2014
  14. 13/16 refs.c: replace the onerr argument in update_ref with a strbuf errRonnie Sahlberg, Nov 7, 2014
  15. 14/16 refs.c: make add_packed_ref return an error instead of calling dieRonnie Sahlberg, Nov 7, 2014
  16. 15/16 refs.c: make lock_packed_refs take an err argumentRonnie Sahlberg, Nov 7, 2014
  17. 16/16 refs.c: add an err argument to pack_refsRonnie Sahlberg, Nov 7, 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.