refs/reftable: reload locked stack when preparing transaction

When starting a reftable transaction we lock all stacks we are about to modify. While it may happen that the stack is out-of-date at this point in time we don't really care: transactional updates encode the expected state of a certain reference, so all that we really want to verify is that the _current_ value matches that expected state. Pass `REFTABLE_STACK_NEW_ADDITION_RELOAD` when locking the stack such that an out-of-date stack will be reloaded after having been locked. This change is safe because all verifications of the expected state happen after this step anyway. Add a testcase that verifies that many writers are now able to write to the stack concurrently without failures and with a deterministic end result. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Sep 24, 2024 at 07:33 UTC 6241ce217046cc77d306bda06ad80ac1973b4932
2 files changed +33 -1
refs/reftable-backend.c
+2 -1
index 6ca00627dd..efe2b0258c 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -770,7 +770,8 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out, if (ret) return ret; - ret = reftable_stack_new_addition(&addition, stack, 0); + ret = reftable_stack_new_addition(&addition, stack, + REFTABLE_STACK_NEW_ADDITION_RELOAD); if (ret) { if (ret == REFTABLE_LOCK_ERROR) strbuf_addstr(err, "cannot lock references");
t/t0610-reftable-basics.sh
+31
index 62da3e3782..2d951c8ceb 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -450,6 +450,37 @@ test_expect_success 'ref transaction: retry acquiring tables.list lock' ' ) ' +test_expect_success 'ref transaction: many concurrent writers' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + # Set a high timeout such that a busy CI machine will not abort + # early. 10 seconds should hopefully be ample of time to make + # this non-flaky. + git config set reftable.lockTimeout 10000 && + test_commit --no-tag initial && + + head=$(git rev-parse HEAD) && + for i in $(test_seq 100) + do + printf "%s commit\trefs/heads/branch-%s\n" "$head" "$i" || + return 1 + done >expect && + printf "%s commit\trefs/heads/main\n" "$head" >>expect && + + for i in $(test_seq 100) + do + { git update-ref refs/heads/branch-$i HEAD& } || + return 1 + done && + + wait && + git for-each-ref --sort=v:refname >actual && + test_cmp expect actual + ) +' + test_expect_success 'pack-refs: compacts tables' ' test_when_finished "rm -rf repo" && git init repo &&