Re: [PATCH v2 3/9] reftable/stack: test compaction with already-locked tables

From: Karthik Nayak <karthik.188@gmail.com>
Date: Aug 8, 2024, 10:46 UTC
Message-ID: <CAOLa=ZQJVTspAsjxrQa-WVxX4tnhAd0NfwL2e-MXHvyq5+cwfg@mail.gmail.com>
In-Reply-To: <63e64c8d82783b5d3fc8db189a29d69c844f5465.1722862822.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> We're lacking test coverage for compacting tables when some of the
> tables that we are about to compact are locked. Add two tests that
> exercise this, one for auto-compaction and one for full compaction.
>

So this patch prepares for the upcoming fixes by adding tests which fail compaction. Makes sense.

[snip]

Show 29 quoted lines

> +static void test_reftable_stack_compaction_with_locked_tables(void)
> +{
> +	struct reftable_write_options opts = {
> +		.disable_auto_compact = 1,
> +	};
> +	struct reftable_stack *st = NULL;
> +	struct strbuf buf = STRBUF_INIT;
> +	char *dir = get_tmp_dir(__LINE__);
> +	int err;
> +
> +	err = reftable_new_stack(&st, dir, &opts);
> +	EXPECT_ERR(err);
> +
> +	write_n_ref_tables(st, &opts, 3);
> +	EXPECT(st->merged->stack_len == 3);
> +
> +	/* Lock one of the tables that we're about to compact. */
> +	strbuf_reset(&buf);
> +	strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[1]->name);
> +	write_file_buf(buf.buf, "", 0);
> +
> +	/*
> +	 * Compaction is expected to fail given that we were not able to
> +	 * compact all tables.
> +	 */
> +	err = reftable_stack_compact_all(st, NULL);
> +	EXPECT(err == REFTABLE_LOCK_ERROR);
> +	/* TODO: this is wrong, we should get notified about the failure. */
> +	EXPECT(st->stats.failures == 0);

This is a good catch. The autocompaction code has a wrapper `stack_compact_range_stats` which handles this exact scenario.

[snip]