Subject: Re: [PATCH v2 04/15] reftable/stack: gracefully handle failed auto-compaction due to locks

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 25 Mar 2024 12:22:24 +0100

On Mon, Mar 25, 2024 at 11:02:51AM +0100, Patrick Steinhardt wrote:
[snip]
> 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);
> +}
> +

I forgot to free the `table_path` buffer here. So this needs the
following patch on top:

diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 822e681028..7b2a8b1afd 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -382,6 +382,7 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(void)
        EXPECT(st->stats.failures == 1);

        reftable_stack_destroy(st);
+       strbuf_release(&table_path);
        clear_dir(dir);
 }

Patrick