[PATCH v2 04/15] reftable/stack: gracefully handle failed auto-compaction due to locks
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Mar 25, 2024, 10:02 UTC
- Message-ID
- <50a3c37f92a28876c0db24e515826485c863ecc3.1711360631.git.ps@pks.im>
- In-Reply-To
- <cover.1711360631.git.ps@pks.im>
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> --- reftable/stack.c | 13 +++++++++++- reftable/stack_test.c | 43 ++++++++++++++++++++++++++++++++++++++ t/t0610-reftable-basics.sh | 20 ++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/reftable/stack.c b/reftable/stack.c index 79856b6565..dde50b61d6 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -680,8 +680,19 @@ int reftable_addition_commit(struct reftable_addition *add) if (err) goto done; - if (!add->stack->disable_auto_compact) + if (!add->stack->disable_auto_compact) { + /* + * Auto-compact the stack to keep the number of tables in + * control. It is possible that a concurrent writer is already + * trying to compact parts of the stack, which would lead to a + * `REFTABLE_LOCK_ERROR` because parts of the stack are locked + * already. This is a benign error though, so we ignore it. + */ err = reftable_stack_auto_compact(add->stack); + if (err < 0 && err != REFTABLE_LOCK_ERROR) + goto done; + err = 0; + } done: reftable_addition_close(add); diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 2c3540d9e6..822e681028 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -343,6 +343,48 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_auto_compaction_fails_gracefully(void) +{ + struct reftable_ref_record ref = { + .refname = "refs/heads/master", + .update_index = 1, + .value_type = REFTABLE_REF_VAL1, + .value.val1 = {0x01}, + }; + struct reftable_write_options cfg = {0}; + struct reftable_stack *st; + struct strbuf table_path = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err; + + err = reftable_new_stack(&st, dir, cfg); + EXPECT_ERR(err); + + err = reftable_stack_add(st, write_test_ref, &ref); + EXPECT_ERR(err); + EXPECT(st->merged->stack_len == 1); + EXPECT(st->stats.attempts == 0); + EXPECT(st->stats.failures == 0); + + /* + * Lock the newly written table such that it cannot be compacted. + * Adding a new table to the stack should not be impacted by this, even + * though auto-compaction will now fail. + */ + strbuf_addf(&table_path, "%s/%s.lock", dir, st->readers[0]->name); + write_file_buf(table_path.buf, "", 0); + + ref.update_index = 2; + err = reftable_stack_add(st, write_test_ref, &ref); + EXPECT_ERR(err); + EXPECT(st->merged->stack_len == 2); + EXPECT(st->stats.attempts == 1); + EXPECT(st->stats.failures == 1); + + reftable_stack_destroy(st); + clear_dir(dir); +} + static void test_reftable_stack_validate_refname(void) { struct reftable_write_options cfg = { 0 }; @@ -1085,6 +1127,7 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_tombstone); RUN_TEST(test_reftable_stack_transaction_api); RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction); + RUN_TEST(test_reftable_stack_auto_compaction_fails_gracefully); RUN_TEST(test_reftable_stack_update_index_check); RUN_TEST(test_reftable_stack_uptodate); RUN_TEST(test_reftable_stack_validate_refname); diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index 686781192e..5f2f9baa9b 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -340,6 +340,26 @@ test_expect_success 'ref transaction: empty transaction in empty repo' ' EOF ' +test_expect_success 'ref transaction: fails gracefully when auto compaction fails' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + + test_commit A && + for i in $(test_seq 10) + do + git branch branch-$i && + for table in .git/reftable/*.ref + do + touch "$table.lock" || exit 1 + done || + exit 1 + done && + test_line_count = 13 .git/reftable/tables.list + ) +' + test_expect_success 'pack-refs: compacts tables' ' test_when_finished "rm -rf repo" && git init repo &&
-- 2.44.GIT