@@ -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);
@@ -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);
@@ -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 &&
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(-)