refs: introduce "initial" transaction flag
There are two different ways to commit a transaction: - `ref_transaction_commit()` can be used to commit a regular transaction and is what almost every caller wants. - `initial_ref_transaction_commit()` can be used when it is known that the ref store that the transaction is committed for is empty and when there are no concurrent processes. This is used when cloning a new repository. Implementing this via two separate functions has a couple of downsides. First, every reference backend needs to implement a separate callback even in the case where they don't special-case the initial transaction. Second, backends are basically forced to reimplement the whole logic for how to commit the transaction like the "files" backend does, even though backends may wish to only tweak certain behaviour of a "normal" commit. Third, it is awkward that callers must never prepare the transaction as this is somewhat different than how a transaction typically works. Refactor the code such that we instead mark initial transactions via a separate flag when starting the transaction. This addresses all of the mentioned painpoints, where the most important part is that it will allow backends to have way more leeway in how exactly they want to handle the initial transaction. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Nov 20, 2024 at 08:51 UTC
1c299d03e5551847533019aa32863d2cbe589c7f
8 files changed
+29
-68
builtin/clone.c
+2
-2
@@ -574,7 +574,7 @@ static void write_remote_refs(const struct ref *local_refs)
574
struct strbuf err = STRBUF_INIT;
575
576
t = ref_store_transaction_begin(get_main_ref_store(the_repository),
577
- 0, &err);
577
+ REF_TRANSACTION_FLAG_INITIAL, &err);
578
if (!t)
579
die("%s", err.buf);
580
@@ -586,7 +586,7 @@ static void write_remote_refs(const struct ref *local_refs)
586
die("%s", err.buf);
587
}
588
589
- if (initial_ref_transaction_commit(t, &err))
589
+ if (ref_transaction_commit(t, &err))
590
die("%s", err.buf);
591
592
strbuf_release(&err);
refs.c
+1
-9
@@ -2315,7 +2315,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
2315
}
2316
2317
ret = refs->be->transaction_finish(refs, transaction, err);
2318
- if (!ret)
2318
+ if (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL))
2319
run_transaction_hook(transaction, "committed");
2320
return ret;
2321
}
@@ -2486,14 +2486,6 @@ int refs_reflog_expire(struct ref_store *refs,
2486
cleanup_fn, policy_cb_data);
2487
}
2488
2489
-int initial_ref_transaction_commit(struct ref_transaction *transaction,
2490
- struct strbuf *err)
2491
-{
2492
- struct ref_store *refs = transaction->ref_store;
2493
-
2494
- return refs->be->initial_transaction_commit(refs, transaction, err);
2495
-}
2496
-
2489
void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,
2490
ref_transaction_for_each_queued_update_fn cb,
2491
void *cb_data)
refs.h
+18
-19
@@ -214,11 +214,9 @@ char *repo_default_branch_name(struct repository *r, int quiet);
214
*
215
* Or
216
*
217
- * - Call `initial_ref_transaction_commit()` if the ref database is
218
- * known to be empty and have no other writers (e.g. during
219
- * clone). This is likely to be much faster than
220
- * `ref_transaction_commit()`. `ref_transaction_prepare()` should
221
- * *not* be called before `initial_ref_transaction_commit()`.
217
+ * - Call `ref_transaction_begin()` with REF_TRANSACTION_FLAG_INITIAL if the
218
+ * ref database is known to be empty and have no other writers (e.g. during
219
+ * clone). This is likely to be much faster than without the flag.
220
*
221
* - Then finally, call `ref_transaction_free()` to free the
222
* `ref_transaction` data structure.
@@ -579,6 +577,21 @@ enum action_on_err {
577
UPDATE_REFS_QUIET_ON_ERR
578
};
579
580
+enum ref_transaction_flag {
581
+ /*
582
+ * The ref transaction is part of the initial creation of the ref store
583
+ * and can thus assume that the ref store is completely empty. This
584
+ * allows the backend to perform the transaction more efficiently by
585
+ * skipping certain checks.
586
+ *
587
+ * It is a bug to set this flag when there might be other processes
588
+ * accessing the repository or if there are existing references that
589
+ * might conflict with the ones being created. All old_oid values must
590
+ * either be absent or null_oid.
591
+ */
592
+ REF_TRANSACTION_FLAG_INITIAL = (1 << 0),
593
+};
594
+
595
/*
596
* Begin a reference transaction. The reference transaction must
597
* be freed by calling ref_transaction_free().
@@ -798,20 +811,6 @@ int ref_transaction_commit(struct ref_transaction *transaction,
811
int ref_transaction_abort(struct ref_transaction *transaction,
812
struct strbuf *err);
813
801
-/*
802
- * Like ref_transaction_commit(), but optimized for creating
803
- * references when originally initializing a repository (e.g., by "git
804
- * clone"). It writes the new references directly to packed-refs
805
- * without locking the individual references.
806
- *
807
- * It is a bug to call this function when there might be other
808
- * processes accessing the repository or if there are existing
809
- * references that might conflict with the ones being created. All
810
- * old_oid values must either be absent or null_oid.
811
- */
812
-int initial_ref_transaction_commit(struct ref_transaction *transaction,
813
- struct strbuf *err);
814
-
814
/*
815
* Execute the given callback function for each of the reference updates which
816
* have been queued in the given transaction. `old_oid` and `new_oid` may be
refs/debug.c
-13
@@ -118,18 +118,6 @@ static int debug_transaction_abort(struct ref_store *refs,
118
return res;
119
}
120
121
-static int debug_initial_transaction_commit(struct ref_store *refs,
122
- struct ref_transaction *transaction,
123
- struct strbuf *err)
124
-{
125
- struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
126
- int res;
127
- transaction->ref_store = drefs->refs;
128
- res = drefs->refs->be->initial_transaction_commit(drefs->refs,
129
- transaction, err);
130
- return res;
131
-}
132
-
121
static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
122
{
123
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
@@ -443,7 +431,6 @@ struct ref_storage_be refs_be_debug = {
431
.transaction_prepare = debug_transaction_prepare,
432
.transaction_finish = debug_transaction_finish,
433
.transaction_abort = debug_transaction_abort,
446
- .initial_transaction_commit = debug_initial_transaction_commit,
434
435
.pack_refs = debug_pack_refs,
436
.rename_ref = debug_rename_ref,
refs/files-backend.c
+8
-8
@@ -2781,6 +2781,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
2781
2782
assert(err);
2783
2784
+ if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)
2785
+ goto cleanup;
2786
if (!transaction->nr)
2787
goto cleanup;
2788
@@ -2985,13 +2987,10 @@ static int ref_present(const char *refname, const char *referent UNUSED,
2987
return string_list_has_string(affected_refnames, refname);
2988
}
2989
2988
-static int files_initial_transaction_commit(struct ref_store *ref_store,
2990
+static int files_transaction_finish_initial(struct files_ref_store *refs,
2991
struct ref_transaction *transaction,
2992
struct strbuf *err)
2993
{
2992
- struct files_ref_store *refs =
2993
- files_downcast(ref_store, REF_STORE_WRITE,
2994
- "initial_ref_transaction_commit");
2994
size_t i;
2995
int ret = 0;
2996
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
@@ -2999,8 +2998,8 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
2998
2999
assert(err);
3000
3002
- if (transaction->state != REF_TRANSACTION_OPEN)
3003
- BUG("commit called for transaction that is not open");
3001
+ if (transaction->state != REF_TRANSACTION_PREPARED)
3002
+ BUG("commit called for transaction that is not prepared");
3003
3004
/* Fail if a refname appears more than once in the transaction: */
3005
for (i = 0; i < transaction->nr; i++)
@@ -3063,7 +3062,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
3062
goto cleanup;
3063
}
3064
3066
- if (initial_ref_transaction_commit(packed_transaction, err)) {
3065
+ if (ref_transaction_commit(packed_transaction, err)) {
3066
ret = TRANSACTION_GENERIC_ERROR;
3067
}
3068
@@ -3091,6 +3090,8 @@ static int files_transaction_finish(struct ref_store *ref_store,
3090
3091
assert(err);
3092
3093
+ if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL)
3094
+ return files_transaction_finish_initial(refs, transaction, err);
3095
if (!transaction->nr) {
3096
transaction->state = REF_TRANSACTION_CLOSED;
3097
return 0;
@@ -3617,7 +3618,6 @@ struct ref_storage_be refs_be_files = {
3618
.transaction_prepare = files_transaction_prepare,
3619
.transaction_finish = files_transaction_finish,
3620
.transaction_abort = files_transaction_abort,
3620
- .initial_transaction_commit = files_initial_transaction_commit,
3621
3622
.pack_refs = files_pack_refs,
3623
.rename_ref = files_rename_ref,
refs/packed-backend.c
-8
@@ -1730,13 +1730,6 @@ cleanup:
1730
return ret;
1731
}
1732
1733
-static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED,
1734
- struct ref_transaction *transaction,
1735
- struct strbuf *err)
1736
-{
1737
- return ref_transaction_commit(transaction, err);
1738
-}
1739
-
1733
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
1734
struct pack_refs_opts *pack_opts UNUSED)
1735
{
@@ -1769,7 +1762,6 @@ struct ref_storage_be refs_be_packed = {
1762
.transaction_prepare = packed_transaction_prepare,
1763
.transaction_finish = packed_transaction_finish,
1764
.transaction_abort = packed_transaction_abort,
1772
- .initial_transaction_commit = packed_initial_transaction_commit,
1765
1766
.pack_refs = packed_pack_refs,
1767
.rename_ref = NULL,
refs/refs-internal.h
-1
@@ -666,7 +666,6 @@ struct ref_storage_be {
666
ref_transaction_prepare_fn *transaction_prepare;
667
ref_transaction_finish_fn *transaction_finish;
668
ref_transaction_abort_fn *transaction_abort;
669
- ref_transaction_commit_fn *initial_transaction_commit;
669
670
pack_refs_fn *pack_refs;
671
rename_ref_fn *rename_ref;
refs/reftable-backend.c
-8
@@ -1490,13 +1490,6 @@ done:
1490
return ret;
1491
}
1492
1493
-static int reftable_be_initial_transaction_commit(struct ref_store *ref_store UNUSED,
1494
- struct ref_transaction *transaction,
1495
- struct strbuf *err)
1496
-{
1497
- return ref_transaction_commit(transaction, err);
1498
-}
1499
-
1493
static int reftable_be_pack_refs(struct ref_store *ref_store,
1494
struct pack_refs_opts *opts)
1495
{
@@ -2490,7 +2483,6 @@ struct ref_storage_be refs_be_reftable = {
2483
.transaction_prepare = reftable_be_transaction_prepare,
2484
.transaction_finish = reftable_be_transaction_finish,
2485
.transaction_abort = reftable_be_transaction_abort,
2493
- .initial_transaction_commit = reftable_be_initial_transaction_commit,
2486
2487
.pack_refs = reftable_be_pack_refs,
2488
.rename_ref = reftable_be_rename_ref,