In the future, compound reference stores will sometimes need to modify references in two different reference stores at the same time, meaning that a single logical reference transaction might have to be implemented as two internal sub-transactions. They won't want to call `ref_transaction_commit()` for the two sub-transactions one after the other, because that wouldn't be atomic (the first commit could succeed and the second one fail). Instead, they will want to prepare both sub-transactions (i.e., obtain any necessary locks and do any pre-checks), and only if both prepare steps succeed, then commit both sub-transactions. Start preparing for that day by adding a new, optional `ref_transaction_prepare()` step to the reference transaction sequence, which obtains the locks and does any prechecks, reporting any errors that occur. Also add a `ref_transaction_abort()` function that can be used to abort a sub-transaction even if it has already been prepared. That is on the side of the public-facing API. On the side of the `ref_store` VTABLE, get rid of `transaction_commit` and instead add methods `transaction_prepare`, `transaction_finish`, and `transaction_abort`. A `ref_transaction_commit()` now basically calls methods `transaction_prepare` then `transaction_finish`. Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx> --- refs.c | 74 ++++++++++++++++++++++++++++-- refs.h | 124 ++++++++++++++++++++++++++++++++++++++++----------- refs/files-backend.c | 63 ++++++++++++++++++++------ refs/refs-internal.h | 45 ++++++++++++++----- 4 files changed, 253 insertions(+), 53 deletions(-) diff --git a/refs.c b/refs.c index f8f41ffb04..b3860a9e33 100644 --- a/refs.c +++ b/refs.c @@ -853,6 +853,19 @@ void ref_transaction_free(struct ref_transaction *transaction) if (!transaction) return; + switch (transaction->state) { + case REF_TRANSACTION_OPEN: + case REF_TRANSACTION_CLOSED: + /* OK */ + break; + case REF_TRANSACTION_PREPARED: + die("BUG: free called on a prepared reference transaction"); + break; + default: + die("BUG: unexpected reference transaction state"); + break; + } + for (i = 0; i < transaction->nr; i++) { free(transaction->updates[i]->msg); free(transaction->updates[i]); @@ -1689,8 +1702,8 @@ int create_symref(const char *ref_target, const char *refs_heads_master, refs_heads_master, logmsg); } -int ref_transaction_commit(struct ref_transaction *transaction, - struct strbuf *err) +int ref_transaction_prepare(struct ref_transaction *transaction, + struct strbuf *err) { struct ref_store *refs = transaction->ref_store; @@ -1698,6 +1711,9 @@ int ref_transaction_commit(struct ref_transaction *transaction, case REF_TRANSACTION_OPEN: /* Good. */ break; + case REF_TRANSACTION_PREPARED: + die("BUG: prepare called twice on reference transaction"); + break; case REF_TRANSACTION_CLOSED: die("BUG: prepare called on a closed reference transaction"); break; @@ -1712,7 +1728,59 @@ int ref_transaction_commit(struct ref_transaction *transaction, return -1; } - return refs->be->transaction_commit(refs, transaction, err); + return refs->be->transaction_prepare(refs, transaction, err); +} + +int ref_transaction_abort(struct ref_transaction *transaction, + struct strbuf *err) +{ + struct ref_store *refs = transaction->ref_store; + int ret = 0; + + switch (transaction->state) { + case REF_TRANSACTION_OPEN: + /* No need to abort explicitly. */ + break; + case REF_TRANSACTION_PREPARED: + ret = refs->be->transaction_abort(refs, transaction, err); + break; + case REF_TRANSACTION_CLOSED: + die("BUG: abort called on a closed reference transaction"); + break; + default: + die("BUG: unexpected reference transaction state"); + break; + } + + ref_transaction_free(transaction); + return ret; +} + +int ref_transaction_commit(struct ref_transaction *transaction, + struct strbuf *err) +{ + struct ref_store *refs = transaction->ref_store; + int ret; + + switch (transaction->state) { + case REF_TRANSACTION_OPEN: + /* Need to prepare first. */ + ret = ref_transaction_prepare(transaction, err); + if (ret) + return ret; + break; + case REF_TRANSACTION_PREPARED: + /* Fall through to finish. */ + break; + case REF_TRANSACTION_CLOSED: + die("BUG: commit called on a closed reference transaction"); + break; + default: + die("BUG: unexpected reference transaction state"); + break; + } + + return refs->be->transaction_finish(refs, transaction, err); } int refs_verify_refname_available(struct ref_store *refs, diff --git a/refs.h b/refs.h index b62722fb81..4be14c4b3c 100644 --- a/refs.h +++ b/refs.h @@ -143,30 +143,71 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref); int dwim_log(const char *str, int len, unsigned char *sha1, char **ref); /* - * A ref_transaction represents a collection of ref updates - * that should succeed or fail together. + * A ref_transaction represents a collection of reference updates that + * should succeed or fail together. * * Calling sequence * ---------------- + * * - Allocate and initialize a `struct ref_transaction` by calling * `ref_transaction_begin()`. * - * - List intended ref updates by calling functions like - * `ref_transaction_update()` and `ref_transaction_create()`. - * - * - Call `ref_transaction_commit()` to execute the transaction. - * If this succeeds, the ref updates will have taken place and - * the transaction cannot be rolled back. - * - * - Instead of `ref_transaction_commit`, use - * `initial_ref_transaction_commit()` if the ref database is known - * to be empty (e.g. during clone). This is likely to be much - * faster. - * - * - At any time call `ref_transaction_free()` to discard the - * transaction and free associated resources. In particular, - * this rolls back the transaction if it has not been - * successfully committed. + * - Specify the intended ref updates by calling one or more of the + * following functions: + * - `ref_transaction_update()` + * - `ref_transaction_create()` + * - `ref_transaction_delete()` + * - `ref_transaction_verify()` + * + * - Then either: + * + * - Optionally call `ref_transaction_prepare()` to prepare the + * transaction. This locks all references, checks preconditions, + * etc. but doesn't finalize anything. If this step fails, the + * transaction has been closed and can only be freed. If this step + * succeeds, then `ref_transaction_commit()` is almost certain to + * succeed. However, you can still call `ref_transaction_abort()` + * if you decide not to commit the transaction after all. + * + * - Call `ref_transaction_commit()` to execute the transaction, + * make the changes permanent, and release all locks. If you + * haven't already called `ref_transaction_prepare()`, then + * `ref_transaction_commit()` calls it for you. + * + * Or + * + * - Call `initial_ref_transaction_commit()` if the ref database is + * known to be empty and have no other writers (e.g. during + * clone). This is likely to be much faster than + * `ref_transaction_commit()`. `ref_transaction_prepare()` should + * *not* be called before `initial_ref_transaction_commit()`. + * + * - Then finally, call `ref_transaction_free()` to free the + * `ref_transaction` data structure. + * + * At any time before calling `ref_transaction_commit()`, you can call + * `ref_transaction_abort()` to abort the transaction, rollback any + * locks, and free any associated resources (including the + * `ref_transaction` data structure). + * + * Putting it all together, a complete reference update looks like + * + * struct ref_transaction *transaction; + * struct strbuf err = STRBUF_INIT; + * int ret = 0; + * + * transaction = ref_store_transaction_begin(refs, &err); + * if (!transaction || + * ref_transaction_update(...) || + * ref_transaction_create(...) || + * ...etc... || + * ref_transaction_commit(transaction, &err)) { + * error("%s", err.buf); + * ret = -1; + * } + * ref_transaction_free(transaction); + * strbuf_release(&err); + * return ret; * * Error handling * -------------- @@ -183,8 +224,9 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **ref); * ------- * * Note that no locks are taken, and no refs are read, until - * `ref_transaction_commit` is called. So `ref_transaction_verify` - * won't report a verification failure until the commit is attempted. + * `ref_transaction_prepare()` or `ref_transaction_commit()` is + * called. So, for example, `ref_transaction_verify()` won't report a + * verification failure until the commit is attempted. */ struct ref_transaction; @@ -523,19 +565,47 @@ int ref_transaction_verify(struct ref_transaction *transaction, unsigned int flags, struct strbuf *err); -/* - * Commit all of the changes that have been queued in transaction, as - * atomically as possible. - * - * Returns 0 for success, or one of the below error codes for errors. - */ /* Naming conflict (for example, the ref names A and A/B conflict). */ #define TRANSACTION_NAME_CONFLICT -1 /* All other errors. */ #define TRANSACTION_GENERIC_ERROR -2 + +/* + * Perform the preparatory stages of commiting `transaction`. Acquire + * any needed locks, check preconditions, etc.; basically, do as much + * as possible to ensure that the transaction will be able to go + * through, stopping just short of making any irrevocable or + * user-visible changes. The updates that this function prepares can + * be finished up by calling `ref_transaction_commit()` or rolled back + * by calling `ref_transaction_abort()`. + * + * On success, return 0 and leave the transaction in "prepared" state. + * On failure, abort the transaction, write an error message to `err`, + * and return one of the `TRANSACTION_*` constants. + * + * Callers who don't need such fine-grained control over commiting + * reference transactions should just call `ref_transaction_commit()`. + */ +int ref_transaction_prepare(struct ref_transaction *transaction, + struct strbuf *err); + +/* + * Commit all of the changes that have been queued in transaction, as + * atomically as possible. On success, return 0 and leave the + * transaction in "closed" state. On failure, roll back the + * transaction, write an error message to `err`, and return one of the + * `TRANSACTION_*` constants + */ int ref_transaction_commit(struct ref_transaction *transaction, struct strbuf *err); +/* + * Abort `transaction`, which has been begun and possibly prepared, + * but not yet committed. + */ +int ref_transaction_abort(struct ref_transaction *transaction, + struct strbuf *err); + /* * Like ref_transaction_commit(), but optimized for creating * references when originally initializing a repository (e.g., by "git @@ -551,7 +621,7 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction, struct strbuf *err); /* - * Free an existing transaction and all associated data. + * Free `*transaction` and all associated data. */ void ref_transaction_free(struct ref_transaction *transaction); diff --git a/refs/files-backend.c b/refs/files-backend.c index a4261d4683..19842d2e56 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2855,22 +2855,19 @@ static void files_transaction_cleanup(struct ref_transaction *transaction) transaction->state = REF_TRANSACTION_CLOSED; } -static int files_transaction_commit(struct ref_store *ref_store, - struct ref_transaction *transaction, - struct strbuf *err) +static int files_transaction_prepare(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE, - "ref_transaction_commit"); + "ref_transaction_prepare"); size_t i; int ret = 0; - struct string_list refs_to_delete = STRING_LIST_INIT_NODUP; - struct string_list_item *ref_to_delete; struct string_list affected_refnames = STRING_LIST_INIT_NODUP; char *head_ref = NULL; int head_type; struct object_id head_oid; - struct strbuf sb = STRBUF_INIT; assert(err); @@ -2934,6 +2931,8 @@ static int files_transaction_commit(struct ref_store *ref_store, * that new values are valid, and write new values to the * lockfiles, ready to be activated. Only keep one lockfile * open at a time to avoid running out of file descriptors. + * Note that lock_ref_for_update() might append more updates + * to the transaction. */ for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; @@ -2941,7 +2940,38 @@ static int files_transaction_commit(struct ref_store *ref_store, ret = lock_ref_for_update(refs, update, transaction, head_ref, &affected_refnames, err); if (ret) - goto cleanup; + break; + } + +cleanup: + free(head_ref); + string_list_clear(&affected_refnames, 0); + + if (ret) + files_transaction_cleanup(transaction); + else + transaction->state = REF_TRANSACTION_PREPARED; + + return ret; +} + +static int files_transaction_finish(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) +{ + struct files_ref_store *refs = + files_downcast(ref_store, 0, "ref_transaction_finish"); + size_t i; + int ret = 0; + struct string_list refs_to_delete = STRING_LIST_INIT_NODUP; + struct string_list_item *ref_to_delete; + struct strbuf sb = STRBUF_INIT; + + assert(err); + + if (!transaction->nr) { + transaction->state = REF_TRANSACTION_CLOSED; + return 0; } /* Perform updates first so live commits remain referenced */ @@ -3022,7 +3052,6 @@ static int files_transaction_commit(struct ref_store *ref_store, cleanup: files_transaction_cleanup(transaction); - strbuf_release(&sb); for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; @@ -3039,13 +3068,19 @@ static int files_transaction_commit(struct ref_store *ref_store, } } + strbuf_release(&sb); string_list_clear(&refs_to_delete, 0); - free(head_ref); - string_list_clear(&affected_refnames, 0); - return ret; } +static int files_transaction_abort(struct ref_store *ref_store, + struct ref_transaction *transaction, + struct strbuf *err) +{ + files_transaction_cleanup(transaction); + return 0; +} + static int ref_present(const char *refname, const struct object_id *oid, int flags, void *cb_data) { @@ -3316,7 +3351,9 @@ struct ref_storage_be refs_be_files = { "files", files_ref_store_create, files_init_db, - files_transaction_commit, + files_transaction_prepare, + files_transaction_finish, + files_transaction_abort, files_initial_transaction_commit, files_pack_refs, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 95edf6f234..4d3dd17f9f 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -185,17 +185,27 @@ struct ref_update *ref_transaction_add_update( /* * Transaction states. - * OPEN: The transaction is in a valid state and can accept new updates. - * An OPEN transaction can be committed. - * CLOSED: A closed transaction is no longer active and no other operations - * than free can be used on it in this state. - * A transaction can either become closed by successfully committing - * an active transaction or if there is a failure while building - * the transaction thus rendering it failed/inactive. + * + * OPEN: The transaction is initialized and new updates can still be + * added to it. An OPEN transaction can be prepared, + * committed, freed, or aborted (freeing and aborting an open + * transaction are equivalent). + * + * PREPARED: ref_transaction_prepare(), which locks all of the + * references involved in the update and checks that the + * update has no errors, has been called successfully for the + * transaction. A PREPARED transaction can be committed or + * aborted. + * + * CLOSED: The transaction is no longer active. A transaction becomes + * CLOSED if there is a failure while building the transaction + * or if a transaction is committed or aborted. A CLOSED + * transaction can only be freed. */ enum ref_transaction_state { - REF_TRANSACTION_OPEN = 0, - REF_TRANSACTION_CLOSED = 1 + REF_TRANSACTION_OPEN = 0, + REF_TRANSACTION_PREPARED = 1, + REF_TRANSACTION_CLOSED = 2 }; /* @@ -497,6 +507,18 @@ typedef struct ref_store *ref_store_init_fn(const char *gitdir, typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err); +typedef int ref_transaction_prepare_fn(struct ref_store *refs, + struct ref_transaction *transaction, + struct strbuf *err); + +typedef int ref_transaction_finish_fn(struct ref_store *refs, + struct ref_transaction *transaction, + struct strbuf *err); + +typedef int ref_transaction_abort_fn(struct ref_store *refs, + struct ref_transaction *transaction, + struct strbuf *err); + typedef int ref_transaction_commit_fn(struct ref_store *refs, struct ref_transaction *transaction, struct strbuf *err); @@ -600,7 +622,10 @@ struct ref_storage_be { const char *name; ref_store_init_fn *init; ref_init_db_fn *init_db; - ref_transaction_commit_fn *transaction_commit; + + ref_transaction_prepare_fn *transaction_prepare; + ref_transaction_finish_fn *transaction_finish; + ref_transaction_abort_fn *transaction_abort; ref_transaction_commit_fn *initial_transaction_commit; pack_refs_fn *pack_refs; -- 2.11.0