reftable/stack: gracefully handle failed auto-compaction due to locks
Whenever we commit a new table to the reftable stack we will end up invoking auto-compaction of the stack to keep the total number of tables at bay. This auto-compaction may fail though in case at least one of the tables which we are about to compact is locked. This is indicated by the compaction function returning `REFTABLE_LOCK_ERROR`. We do not handle this case though, and thus bubble that return value up the calling chain, which will ultimately cause a failure. Fix this bug by ignoring `REFTABLE_LOCK_ERROR`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Mar 25, 2024 at 11:02 UTC
a2f711ade0c4816a59155d72559cbc4759cd4699
3 files changed
+76
-1
reftable/stack.c
+12
-1
@@ -680,8 +680,19 @@ int reftable_addition_commit(struct reftable_addition *add)
680
if (err)
681
goto done;
682
683
- if (!add->stack->disable_auto_compact)
683
+ if (!add->stack->disable_auto_compact) {
684
+ /*
685
+ * Auto-compact the stack to keep the number of tables in
686
+ * control. It is possible that a concurrent writer is already
687
+ * trying to compact parts of the stack, which would lead to a
688
+ * `REFTABLE_LOCK_ERROR` because parts of the stack are locked
689
+ * already. This is a benign error though, so we ignore it.
690
+ */
691
err = reftable_stack_auto_compact(add->stack);
692
+ if (err < 0 && err != REFTABLE_LOCK_ERROR)
693
+ goto done;
694
+ err = 0;
695
+ }
696
697
done:
698
reftable_addition_close(add);
reftable/stack_test.c
+44
@@ -343,6 +343,49 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
343
clear_dir(dir);
344
}
345
346
+static void test_reftable_stack_auto_compaction_fails_gracefully(void)
347
+{
348
+ struct reftable_ref_record ref = {
349
+ .refname = "refs/heads/master",
350
+ .update_index = 1,
351
+ .value_type = REFTABLE_REF_VAL1,
352
+ .value.val1 = {0x01},
353
+ };
354
+ struct reftable_write_options cfg = {0};
355
+ struct reftable_stack *st;
356
+ struct strbuf table_path = STRBUF_INIT;
357
+ char *dir = get_tmp_dir(__LINE__);
358
+ int err;
359
+
360
+ err = reftable_new_stack(&st, dir, cfg);
361
+ EXPECT_ERR(err);
362
+
363
+ err = reftable_stack_add(st, write_test_ref, &ref);
364
+ EXPECT_ERR(err);
365
+ EXPECT(st->merged->stack_len == 1);
366
+ EXPECT(st->stats.attempts == 0);
367
+ EXPECT(st->stats.failures == 0);
368
+
369
+ /*
370
+ * Lock the newly written table such that it cannot be compacted.
371
+ * Adding a new table to the stack should not be impacted by this, even
372
+ * though auto-compaction will now fail.
373
+ */
374
+ strbuf_addf(&table_path, "%s/%s.lock", dir, st->readers[0]->name);
375
+ write_file_buf(table_path.buf, "", 0);
376
+
377
+ ref.update_index = 2;
378
+ err = reftable_stack_add(st, write_test_ref, &ref);
379
+ EXPECT_ERR(err);
380
+ EXPECT(st->merged->stack_len == 2);
381
+ EXPECT(st->stats.attempts == 1);
382
+ EXPECT(st->stats.failures == 1);
383
+
384
+ reftable_stack_destroy(st);
385
+ strbuf_release(&table_path);
386
+ clear_dir(dir);
387
+}
388
+
389
static void test_reftable_stack_validate_refname(void)
390
{
391
struct reftable_write_options cfg = { 0 };
@@ -1089,6 +1132,7 @@ int stack_test_main(int argc, const char *argv[])
1132
RUN_TEST(test_reftable_stack_tombstone);
1133
RUN_TEST(test_reftable_stack_transaction_api);
1134
RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction);
1135
+ RUN_TEST(test_reftable_stack_auto_compaction_fails_gracefully);
1136
RUN_TEST(test_reftable_stack_update_index_check);
1137
RUN_TEST(test_reftable_stack_uptodate);
1138
RUN_TEST(test_reftable_stack_validate_refname);
t/t0610-reftable-basics.sh
+20
@@ -340,6 +340,26 @@ test_expect_success 'ref transaction: empty transaction in empty repo' '
340
EOF
341
'
342
343
+test_expect_success 'ref transaction: fails gracefully when auto compaction fails' '
344
+ test_when_finished "rm -rf repo" &&
345
+ git init repo &&
346
+ (
347
+ cd repo &&
348
+
349
+ test_commit A &&
350
+ for i in $(test_seq 10)
351
+ do
352
+ git branch branch-$i &&
353
+ for table in .git/reftable/*.ref
354
+ do
355
+ touch "$table.lock" || exit 1
356
+ done ||
357
+ exit 1
358
+ done &&
359
+ test_line_count = 13 .git/reftable/tables.list
360
+ )
361
+'
362
+
363
test_expect_success 'pack-refs: compacts tables' '
364
test_when_finished "rm -rf repo" &&
365
git init repo &&