[PATCH v2 6/7] refs: implement partial reference transaction support
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Feb 25, 2025, 09:29 UTC
- Message-ID
- <20250225-245-partially-atomic-ref-updates-v2-6-cfa3236895d7@gmail.com>
- In-Reply-To
- <20250225-245-partially-atomic-ref-updates-v2-0-cfa3236895d7@gmail.com>
Git's reference transactions are all-or-nothing: either all updates succeed, or none do. While this atomic behavior is generally desirable, it can be suboptimal especially when using the reftable backend, where batching multiple reference updates into a single transaction is more efficient than performing them sequentially.
Introduce partial transaction support with a new flag, 'REF_TRANSACTION_ALLOW_PARTIAL'. When enabled, this flag allows individual reference updates that would typically cause the entire transaction to fail due to non-system-related errors to be marked as rejected while permitting other updates to proceed. Non-system-related errors include issues caused by user-provided input values, whereas system-related errors, such as I/O failures or memory issues, continue to result in a full transaction failure. This approach enhances flexibility while preserving transactional integrity where necessary.
The implementation introduces several key components:
- Add 'rejection_err' field to struct `ref_update` to track failed
updates with failure reason. - Modify reference backends (files, packed, reftable) to handle
partial transactions by using `ref_transaction_set_rejected()`
instead of failing the entire transaction when
`REF_TRANSACTION_ALLOW_PARTIAL` is set. - Add `ref_transaction_for_each_rejected_update()` to let callers
examine which updates were rejected and why.This foundational change enables partial transaction support throughout the reference subsystem. The next commit will expose this capability to users by adding a `--allow-partial` flag to 'git-update-ref(1)', providing both a user-facing feature and a testable implementation.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> --- refs.c | 31 +++++++++++++++++++++++++++++++ refs.h | 22 ++++++++++++++++++++++ refs/files-backend.c | 12 +++++++++++- refs/packed-backend.c | 30 ++++++++++++++++++++++++++++-- refs/refs-internal.h | 13 +++++++++++++ refs/reftable-backend.c | 12 +++++++++++- 6 files changed, 116 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c index f989a46a5a..243c09c368 100644 --- a/refs.c +++ b/refs.c @@ -1211,6 +1211,15 @@ void ref_transaction_free(struct ref_transaction *transaction) free(transaction); } +void ref_transaction_set_rejected(struct ref_transaction *transaction, + size_t update_idx, + enum transaction_error err) +{ + if (update_idx >= transaction->nr) + BUG("trying to set rejection on invalid update index"); + transaction->updates[update_idx]->rejection_err = err; +} + struct ref_update *ref_transaction_add_update( struct ref_transaction *transaction, const char *refname, unsigned int flags, @@ -1236,6 +1245,7 @@ struct ref_update *ref_transaction_add_update( transaction->updates[transaction->nr++] = update; update->flags = flags; + update->rejection_err = TRANSACTION_OK; update->new_target = xstrdup_or_null(new_target); update->old_target = xstrdup_or_null(old_target); @@ -2726,6 +2736,27 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, } } +void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction, + ref_transaction_for_each_rejected_update_fn cb, + void *cb_data) +{ + if (!(transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL)) + return; + + for (size_t i = 0; i < transaction->nr; i++) { + struct ref_update *update = transaction->updates[i]; + + if (!update->rejection_err) + continue; + + cb(update->refname, + (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL, + (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL, + update->old_target, update->new_target, + update->rejection_err, cb_data); + } +} + int refs_delete_refs(struct ref_store *refs, const char *logmsg, struct string_list *refnames, unsigned int flags) { diff --git a/refs.h b/refs.h index 8e9ead174c..e4a6a8218f 100644 --- a/refs.h +++ b/refs.h @@ -675,6 +675,13 @@ enum ref_transaction_flag { * either be absent or null_oid. */ REF_TRANSACTION_FLAG_INITIAL = (1 << 0), + + /* + * The transaction mechanism by default fails all updates if any conflict + * is detected. This flag allows transactions to partially apply updates + * while rejecting updates which do not match the expected state. + */ + REF_TRANSACTION_ALLOW_PARTIAL = (1 << 1), }; /* @@ -905,6 +912,21 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, ref_transaction_for_each_queued_update_fn cb, void *cb_data); +/* + * Execute the given callback function for each of the reference updates which + * have been rejected in the given transaction. + */ +typedef void ref_transaction_for_each_rejected_update_fn(const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + const char *old_target, + const char *new_target, + enum transaction_error err, + void *cb_data); +void ref_transaction_for_each_rejected_update(struct ref_transaction *transaction, + ref_transaction_for_each_rejected_update_fn cb, + void *cb_data); + /* * Free `*transaction` and all associated data. */ diff --git a/refs/files-backend.c b/refs/files-backend.c index 3b0adf8bb2..d0a53c9ace 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2851,8 +2851,18 @@ static int files_transaction_prepare(struct ref_store *ref_store, ret = lock_ref_for_update(refs, update, transaction, head_ref, &refnames_to_check, err); - if (ret) + if (ret) { + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL && + ret != TRANSACTION_GENERIC_ERROR) { + ref_transaction_set_rejected(transaction, i, ret); + + strbuf_setlen(err, 0); + ret = TRANSACTION_OK; + + continue; + } goto cleanup; + } if (update->flags & REF_DELETING && !(update->flags & REF_LOG_ONLY) && diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 75e1ebf67d..0857204213 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1324,10 +1324,11 @@ static int packed_ref_store_remove_on_disk(struct ref_store *ref_store, * remain locked when it is done. */ static enum transaction_error write_with_updates(struct packed_ref_store *refs, - struct string_list *updates, + struct ref_transaction *transaction, struct strbuf *err) { enum transaction_error ret = TRANSACTION_GENERIC_ERROR; + struct string_list *updates = &transaction->refnames; struct ref_iterator *iter = NULL; size_t i; int ok; @@ -1408,6 +1409,14 @@ static enum transaction_error write_with_updates(struct packed_ref_store *refs, "reference already exists", update->refname); ret = TRANSACTION_CREATE_EXISTS; + + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_set_rejected(transaction, i, ret); + strbuf_setlen(err, 0); + ret = 0; + continue; + } + goto error; } else if (!oideq(&update->old_oid, iter->oid)) { strbuf_addf(err, "cannot update ref '%s': " @@ -1416,6 +1425,14 @@ static enum transaction_error write_with_updates(struct packed_ref_store *refs, oid_to_hex(iter->oid), oid_to_hex(&update->old_oid)); ret = TRANSACTION_INCORRECT_OLD_VALUE; + + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_set_rejected(transaction, i, ret); + strbuf_setlen(err, 0); + ret = 0; + continue; + } + goto error; } } @@ -1453,6 +1470,14 @@ static enum transaction_error write_with_updates(struct packed_ref_store *refs, update->refname, oid_to_hex(&update->old_oid)); return TRANSACTION_NONEXISTENT_REF; + + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL) { + ref_transaction_set_rejected(transaction, i, ret); + strbuf_setlen(err, 0); + ret = 0; + continue; + } + goto error; } } @@ -1518,6 +1543,7 @@ static enum transaction_error write_with_updates(struct packed_ref_store *refs, write_error: strbuf_addf(err, "error writing to %s: %s", get_tempfile_path(refs->tempfile), strerror(errno)); + ret = TRANSACTION_GENERIC_ERROR; error: ref_iterator_free(iter); @@ -1676,7 +1702,7 @@ static int packed_transaction_prepare(struct ref_store *ref_store, data->own_lock = 1; } - ret = write_with_updates(refs, &transaction->refnames, err); + ret = write_with_updates(refs, transaction, err); if (ret) goto failure; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index c97045fbed..7196f2d880 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -123,6 +123,11 @@ struct ref_update { */ uint64_t index; + /* + * Used in partial transactions to mark if a given update was rejected. + */ + enum transaction_error rejection_err; + /* * If this ref_update was split off of a symref update via * split_symref_update(), then this member points at that @@ -142,6 +147,14 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, struct object_id *oid, struct strbuf *referent, unsigned int *type, int *failure_errno); +/* + * Mark a given update as rejected with a given reason. To be used in conjuction + * with the `REF_TRANSACTION_ALLOW_PARTIAL` flag to allow partial transactions. + */ +void ref_transaction_set_rejected(struct ref_transaction *transaction, + size_t update_idx, + enum transaction_error err); + /* * Add a ref_update with the specified properties to transaction, and * return a pointer to the new object. This function does not verify diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e1fd9c2de2..83cf8d582b 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1374,8 +1374,18 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, transaction->updates[i], &refnames_to_check, head_type, &head_referent, &referent, err); - if (ret) + if (ret) { + if (transaction->flags & REF_TRANSACTION_ALLOW_PARTIAL && + ret != TRANSACTION_GENERIC_ERROR) { + ref_transaction_set_rejected(transaction, i, ret); + + strbuf_setlen(err, 0); + ret = TRANSACTION_OK; + + continue; + } goto done; + } } string_list_sort(&refnames_to_check);
-- 2.47.2