refs/reftable: always reload stacks when creating lock

When creating a new addition via either `reftable_stack_new_addition()`
or its convenince wrapper `reftable_stack_add()` we:

  1. Create the "tables.list.lock" file.

  2. Verify that the current version of the "tables.list" file is
     up-to-date.

  3. Write the new table records if so.

By default, the second step would cause us to bail out if we see that
there has been a concurrent write to the stack that made our in-memory
copy of the stack out-of-date. This is a safety mechanism to not write
records to the stack based on outdated information.

The downside though is that concurrent writes may now cause us to bail
out, which is not a good user experience. In addition, this isn't even
necessary for us, as Git knows to perform all checks for the old state
of references under the lock. (Well, in all except one case: when we
expire the reflog we first create the log iterator before we create the
lock, but this ordering is fixed as part of this commit.)

Consequently, most writers pass the `REFTABLE_STACK_NEW_ADDITION_RELOAD`
flag. The effect of this flag is that we reload the stack after having
acquired the lock in case the stack is out-of-date. This plugs the race
with concurrent writers, but we continue performing the verifications of
the expected old state to catch actual conflicts in the references we
are about to write.

Adapt the remaining callsites that don't yet pass this flag to do so.
While at it, drop a needless manual reload.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 3f0deab338c2883754d9df2f8a29bb04872f2ed7..66d25411f1e9e39e00f7b690c64373b51f8f7c7e 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1006,10 +1006,6 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out,
        if (!arg) {
                struct reftable_addition *addition;
 
-               ret = reftable_stack_reload(be->stack);
-               if (ret)
-                       return ret;
-
                ret = reftable_stack_new_addition(&addition, be->stack,
                                                  REFTABLE_STACK_NEW_ADDITION_RELOAD);
                if (ret) {
@@ -1960,7 +1956,8 @@ static int reftable_be_rename_ref(struct ref_store *ref_store,
        ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);
        if (ret)
                goto done;
-       ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg, 0);
+       ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,
+                                REFTABLE_STACK_NEW_ADDITION_RELOAD);
 
 done:
        assert(ret != REFTABLE_API_ERROR);
@@ -1989,7 +1986,8 @@ static int reftable_be_copy_ref(struct ref_store *ref_store,
        ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1);
        if (ret)
                goto done;
-       ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg, 0);
+       ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg,
+                                REFTABLE_STACK_NEW_ADDITION_RELOAD);
 
 done:
        assert(ret != REFTABLE_API_ERROR);
@@ -2360,7 +2358,8 @@ static int reftable_be_create_reflog(struct ref_store *ref_store,
                goto done;
        arg.stack = be->stack;
 
-       ret = reftable_stack_add(be->stack, &write_reflog_existence_table, &arg, 0);
+       ret = reftable_stack_add(be->stack, &write_reflog_existence_table, &arg,
+                                REFTABLE_STACK_NEW_ADDITION_RELOAD);
 
 done:
        return ret;
@@ -2431,7 +2430,8 @@ static int reftable_be_delete_reflog(struct ref_store *ref_store,
                return ret;
        arg.stack = be->stack;
 
-       ret = reftable_stack_add(be->stack, &write_reflog_delete_table, &arg, 0);
+       ret = reftable_stack_add(be->stack, &write_reflog_delete_table, &arg,
+                                REFTABLE_STACK_NEW_ADDITION_RELOAD);
 
        assert(ret != REFTABLE_API_ERROR);
        return ret;
@@ -2552,15 +2552,16 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
        if (ret < 0)
                goto done;
 
-       ret = reftable_stack_init_log_iterator(be->stack, &it);
+       ret = reftable_stack_new_addition(&add, be->stack,
+                                         REFTABLE_STACK_NEW_ADDITION_RELOAD);
        if (ret < 0)
                goto done;
 
-       ret = reftable_iterator_seek_log(&it, refname);
+       ret = reftable_stack_init_log_iterator(be->stack, &it);
        if (ret < 0)
                goto done;
 
-       ret = reftable_stack_new_addition(&add, be->stack, 0);
+       ret = reftable_iterator_seek_log(&it, refname);
        if (ret < 0)
                goto done;