reftable/stack: allow locking of outdated stacks
In `reftable_stack_new_addition()` we first lock the stack and then check whether it is still up-to-date. If it is not we return an error to the caller indicating that the stack is outdated. This is overly restrictive in our ref transaction interface though: we lock the stack right before we start to verify the transaction, so we do not really care whether it is outdated or not. What we really want is that the stack is up-to-date after it has been locked so that we can verify queued updates against its current state while we know that it is locked for concurrent modification. Introduce a new flag `REFTABLE_STACK_NEW_ADDITION_RELOAD` that alters the behaviour of `reftable_stack_init_addition()` in this case: when we notice that it is out-of-date we reload it instead of returning an error to the caller. This logic will be wired up in the reftable backend in the next commit. 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
80e7342ea8ecda48bdf034e77c32ac1c5d2bda85
4 files changed
+91
-13
refs/reftable-backend.c
+2
-2
@@ -770,7 +770,7 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out,
770
if (ret)
771
return ret;
772
773
- ret = reftable_stack_new_addition(&addition, stack);
773
+ ret = reftable_stack_new_addition(&addition, stack, 0);
774
if (ret) {
775
if (ret == REFTABLE_LOCK_ERROR)
776
strbuf_addstr(err, "cannot lock references");
@@ -2207,7 +2207,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
2207
if (ret < 0)
2208
goto done;
2209
2210
- ret = reftable_stack_new_addition(&add, stack);
2210
+ ret = reftable_stack_new_addition(&add, stack, 0);
2211
if (ret < 0)
2212
goto done;
2213
reftable/reftable-stack.h
+11
-2
@@ -37,12 +37,21 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st);
37
/* holds a transaction to add tables at the top of a stack. */
38
struct reftable_addition;
39
40
+enum {
41
+ /*
42
+ * Reload the stack when the stack is out-of-date after locking it.
43
+ */
44
+ REFTABLE_STACK_NEW_ADDITION_RELOAD = (1 << 0),
45
+};
46
+
47
/*
48
* returns a new transaction to add reftables to the given stack. As a side
42
- * effect, the ref database is locked.
49
+ * effect, the ref database is locked. Accepts REFTABLE_STACK_NEW_ADDITION_*
50
+ * flags.
51
*/
52
int reftable_stack_new_addition(struct reftable_addition **dest,
45
- struct reftable_stack *st);
53
+ struct reftable_stack *st,
54
+ unsigned int flags);
55
56
/* Adds a reftable to transaction. */
57
int reftable_addition_add(struct reftable_addition *add,
reftable/stack.c
+13
-7
@@ -596,7 +596,8 @@ struct reftable_addition {
596
#define REFTABLE_ADDITION_INIT {0}
597
598
static int reftable_stack_init_addition(struct reftable_addition *add,
599
- struct reftable_stack *st)
599
+ struct reftable_stack *st,
600
+ unsigned int flags)
601
{
602
struct strbuf lock_file_name = STRBUF_INIT;
603
int err;
@@ -626,6 +627,11 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
627
err = stack_uptodate(st);
628
if (err < 0)
629
goto done;
630
+ if (err > 0 && flags & REFTABLE_STACK_NEW_ADDITION_RELOAD) {
631
+ err = reftable_stack_reload_maybe_reuse(add->stack, 1);
632
+ if (err)
633
+ goto done;
634
+ }
635
if (err > 0) {
636
err = REFTABLE_OUTDATED_ERROR;
637
goto done;
@@ -633,9 +639,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
639
640
add->next_update_index = reftable_stack_next_update_index(st);
641
done:
636
- if (err) {
642
+ if (err)
643
reftable_addition_close(add);
638
- }
644
strbuf_release(&lock_file_name);
645
return err;
646
}
@@ -739,13 +744,14 @@ done:
744
}
745
746
int reftable_stack_new_addition(struct reftable_addition **dest,
742
- struct reftable_stack *st)
747
+ struct reftable_stack *st,
748
+ unsigned int flags)
749
{
750
int err = 0;
751
struct reftable_addition empty = REFTABLE_ADDITION_INIT;
752
REFTABLE_CALLOC_ARRAY(*dest, 1);
753
**dest = empty;
748
- err = reftable_stack_init_addition(*dest, st);
754
+ err = reftable_stack_init_addition(*dest, st, flags);
755
if (err) {
756
reftable_free(*dest);
757
*dest = NULL;
@@ -759,7 +765,7 @@ static int stack_try_add(struct reftable_stack *st,
765
void *arg)
766
{
767
struct reftable_addition add = REFTABLE_ADDITION_INIT;
762
- int err = reftable_stack_init_addition(&add, st);
768
+ int err = reftable_stack_init_addition(&add, st, 0);
769
if (err < 0)
770
goto done;
771
@@ -1608,7 +1614,7 @@ static int reftable_stack_clean_locked(struct reftable_stack *st)
1614
int reftable_stack_clean(struct reftable_stack *st)
1615
{
1616
struct reftable_addition *add = NULL;
1611
- int err = reftable_stack_new_addition(&add, st);
1617
+ int err = reftable_stack_new_addition(&add, st, 0);
1618
if (err < 0) {
1619
goto done;
1620
}
t/unit-tests/t-reftable-stack.c
+65
-2
@@ -271,7 +271,7 @@ static void t_reftable_stack_transaction_api(void)
271
272
reftable_addition_destroy(add);
273
274
- err = reftable_stack_new_addition(&add, st);
274
+ err = reftable_stack_new_addition(&add, st, 0);
275
check(!err);
276
277
err = reftable_addition_add(add, write_test_ref, &ref);
@@ -292,6 +292,68 @@ static void t_reftable_stack_transaction_api(void)
292
clear_dir(dir);
293
}
294
295
+static void t_reftable_stack_transaction_with_reload(void)
296
+{
297
+ char *dir = get_tmp_dir(__LINE__);
298
+ struct reftable_stack *st1 = NULL, *st2 = NULL;
299
+ int err;
300
+ struct reftable_addition *add = NULL;
301
+ struct reftable_ref_record refs[2] = {
302
+ {
303
+ .refname = (char *) "refs/heads/a",
304
+ .update_index = 1,
305
+ .value_type = REFTABLE_REF_VAL1,
306
+ .value.val1 = { '1' },
307
+ },
308
+ {
309
+ .refname = (char *) "refs/heads/b",
310
+ .update_index = 2,
311
+ .value_type = REFTABLE_REF_VAL1,
312
+ .value.val1 = { '1' },
313
+ },
314
+ };
315
+ struct reftable_ref_record ref = { 0 };
316
+
317
+ err = reftable_new_stack(&st1, dir, NULL);
318
+ check(!err);
319
+ err = reftable_new_stack(&st2, dir, NULL);
320
+ check(!err);
321
+
322
+ err = reftable_stack_new_addition(&add, st1, 0);
323
+ check(!err);
324
+ err = reftable_addition_add(add, write_test_ref, &refs[0]);
325
+ check(!err);
326
+ err = reftable_addition_commit(add);
327
+ check(!err);
328
+ reftable_addition_destroy(add);
329
+
330
+ /*
331
+ * The second stack is now outdated, which we should notice. We do not
332
+ * create the addition and lock the stack by default, but allow the
333
+ * reload to happen when REFTABLE_STACK_NEW_ADDITION_RELOAD is set.
334
+ */
335
+ err = reftable_stack_new_addition(&add, st2, 0);
336
+ check_int(err, ==, REFTABLE_OUTDATED_ERROR);
337
+ err = reftable_stack_new_addition(&add, st2, REFTABLE_STACK_NEW_ADDITION_RELOAD);
338
+ check(!err);
339
+ err = reftable_addition_add(add, write_test_ref, &refs[1]);
340
+ check(!err);
341
+ err = reftable_addition_commit(add);
342
+ check(!err);
343
+ reftable_addition_destroy(add);
344
+
345
+ for (size_t i = 0; i < ARRAY_SIZE(refs); i++) {
346
+ err = reftable_stack_read_ref(st2, refs[i].refname, &ref);
347
+ check(!err);
348
+ check(reftable_ref_record_equal(&refs[i], &ref, GIT_SHA1_RAWSZ));
349
+ }
350
+
351
+ reftable_ref_record_release(&ref);
352
+ reftable_stack_destroy(st1);
353
+ reftable_stack_destroy(st2);
354
+ clear_dir(dir);
355
+}
356
+
357
static void t_reftable_stack_transaction_api_performs_auto_compaction(void)
358
{
359
char *dir = get_tmp_dir(__LINE__);
@@ -322,7 +384,7 @@ static void t_reftable_stack_transaction_api_performs_auto_compaction(void)
384
*/
385
st->opts.disable_auto_compact = i != n;
386
325
- err = reftable_stack_new_addition(&add, st);
387
+ err = reftable_stack_new_addition(&add, st, 0);
388
check(!err);
389
390
err = reftable_addition_add(add, write_test_ref, &ref);
@@ -1314,6 +1376,7 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
1376
TEST(t_reftable_stack_reload_with_missing_table(), "stack iteration with garbage tables");
1377
TEST(t_reftable_stack_tombstone(), "'tombstone' refs in stack");
1378
TEST(t_reftable_stack_transaction_api(), "update transaction to stack");
1379
+ TEST(t_reftable_stack_transaction_with_reload(), "transaction with reload");
1380
TEST(t_reftable_stack_transaction_api_performs_auto_compaction(), "update transaction triggers auto-compaction");
1381
TEST(t_reftable_stack_update_index_check(), "update transactions with equal update indices");
1382
TEST(t_reftable_stack_uptodate(), "stack must be reloaded before ref update");