diff options
author | Patrick Steinhardt <ps@pks.im> | 2024-03-25 11:02:50 +0100 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2024-03-25 09:54:07 -0700 |
commit | a2f711ade0c4816a59155d72559cbc4759cd4699 (patch) | |
tree | 5c3a8c6423e640d428a4ef88875269bae32a0ef4 /reftable | |
parent | 33358350eb857a5fb273d143dcdfe15ac6d4db5d (diff) | |
download | git-a2f711ade0c4816a59155d72559cbc4759cd4699.tar.gz |
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>
Diffstat (limited to 'reftable')
-rw-r--r-- | reftable/stack.c | 13 | ||||
-rw-r--r-- | reftable/stack_test.c | 44 |
2 files changed, 56 insertions, 1 deletions
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 b0c7041a4f..887f458e5c 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -343,6 +343,49 @@ 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); + strbuf_release(&table_path); + clear_dir(dir); +} + static void test_reftable_stack_validate_refname(void) { struct reftable_write_options cfg = { 0 }; @@ -1089,6 +1132,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); |