refs/files: batch refname availability checks for normal transactions

Same as the "reftable" backend that we have adapted in the preceding commit to use batched refname availability checks we can also do so for the "files" backend. Things are a bit more intricate here though, as we call `refs_verify_refname_available()` in a set of different contexts: 1. `lock_raw_ref()` when it hits either EEXISTS or EISDIR when creating a new reference, mostly to create a nice, user-readable error message. This is nothing we have to care about too much, as we only hit this code path at most once when we hit a conflict. 2. `lock_raw_ref()` when it _could_ create the lockfile to check whether it is conflicting with any packed refs. In the general case, this code path will be hit once for every (successful) reference update. 3. `lock_ref_oid_basic()`, but it is only executed when copying or renaming references or when expiring reflogs. It will thus not be called in contexts where we have many references queued up. 4. `refs_refname_ref_available()`, but again only when copying or renaming references. It is thus not interesting due to the same reason as the previous case. 5. `files_transaction_finish_initial()`, which is only executed when creating a new repository or migrating references. So out of these, only (2) and (5) are viable candidates to use the batched checks. Adapt `lock_raw_ref()` accordingly by queueing up reference names that need to be checked for availability and then checking them after we have processed all updates. This check is done before we (optionally) lock the `packed-refs` file, which is somewhat flawed because it means that the `packed-refs` could still change after the availability check and thus create an undetected conflict. But unconditionally locking the file would change semantics that users are likely to rely on, so we keep the current locking sequence intact, even if it's suboptmial. The refactoring of `files_transaction_finish_initial()` will be done in the next commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 12, 2025 at 16:56 UTC 6c90726bebfd8ec4dd429f1fad9d00112b1b6603
1 file changed +31 -11
refs/files-backend.c
+31 -11
@@ -678,6 +678,7 @@ static void unlock_ref(struct ref_lock *lock)
678 */
679 static int lock_raw_ref(struct files_ref_store *refs,
680 const char *refname, int mustexist,
681 + struct string_list *refnames_to_check,
682 const struct string_list *extras,
683 struct ref_lock **lock_p,
684 struct strbuf *referent,
@@ -855,16 +856,11 @@ retry:
856 }
857
858 /*
858 - * If the ref did not exist and we are creating it,
859 - * make sure there is no existing packed ref that
860 - * conflicts with refname:
859 + * If the ref did not exist and we are creating it, we have to
860 + * make sure there is no existing packed ref that conflicts
861 + * with refname. This check is deferred so that we can batch it.
862 */
862 - if (refs_verify_refname_available(
863 - refs->packed_ref_store, refname,
864 - extras, NULL, 0, err)) {
865 - ret = TRANSACTION_NAME_CONFLICT;
866 - goto error_return;
867 - }
863 + string_list_append(refnames_to_check, refname);
864 }
865
866 ret = 0;
@@ -2569,6 +2565,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2565 struct ref_update *update,
2566 struct ref_transaction *transaction,
2567 const char *head_ref,
2568 + struct string_list *refnames_to_check,
2569 struct string_list *affected_refnames,
2570 struct strbuf *err)
2571 {
@@ -2597,7 +2594,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2594 lock->count++;
2595 } else {
2596 ret = lock_raw_ref(refs, update->refname, mustexist,
2600 - affected_refnames,
2597 + refnames_to_check, affected_refnames,
2598 &lock, &referent,
2599 &update->type, err);
2600 if (ret) {
@@ -2811,6 +2808,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
2808 size_t i;
2809 int ret = 0;
2810 struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
2811 + struct string_list refnames_to_check = STRING_LIST_INIT_NODUP;
2812 char *head_ref = NULL;
2813 int head_type;
2814 struct files_transaction_backend_data *backend_data;
@@ -2898,7 +2896,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
2896 struct ref_update *update = transaction->updates[i];
2897
2898 ret = lock_ref_for_update(refs, update, transaction,
2901 - head_ref, &affected_refnames, err);
2899 + head_ref, &refnames_to_check,
2900 + &affected_refnames, err);
2901 if (ret)
2902 goto cleanup;
2903
@@ -2930,6 +2929,26 @@ static int files_transaction_prepare(struct ref_store *ref_store,
2929 }
2930 }
2931
2932 + /*
2933 + * Verify that none of the loose reference that we're about to write
2934 + * conflict with any existing packed references. Ideally, we'd do this
2935 + * check after the packed-refs are locked so that the file cannot
2936 + * change underneath our feet. But introducing such a lock now would
2937 + * probably do more harm than good as users rely on there not being a
2938 + * global lock with the "files" backend.
2939 + *
2940 + * Another alternative would be to do the check after the (optional)
2941 + * lock, but that would extend the time we spend in the globally-locked
2942 + * state.
2943 + *
2944 + * So instead, we accept the race for now.
2945 + */
2946 + if (refs_verify_refnames_available(refs->packed_ref_store, &refnames_to_check,
2947 + &affected_refnames, NULL, 0, err)) {
2948 + ret = TRANSACTION_NAME_CONFLICT;
2949 + goto cleanup;
2950 + }
2951 +
2952 if (packed_transaction) {
2953 if (packed_refs_lock(refs->packed_ref_store, 0, err)) {
2954 ret = TRANSACTION_GENERIC_ERROR;
@@ -2972,6 +2991,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
2991 cleanup:
2992 free(head_ref);
2993 string_list_clear(&affected_refnames, 0);
2994 + string_list_clear(&refnames_to_check, 0);
2995
2996 if (ret)
2997 files_transaction_cleanup(refs, transaction);