Re: [PATCH v2 2/3] reftable/stack: allow locking of outdated stacks

[Patrick Steinhardt <ps@pks.im>]

On Wed, Sep 18, 2024 at 07:26:49PM +1000, James Liu wrote:
> I just want to check my understanding of this test, since I think it's
> the first time I've reviewed anything using this test harness:
> 
> On Wed Sep 18, 2024 at 2:32 PM AEST, Patrick Steinhardt wrote:
> > diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
> > index d62a9c1bed5..a37cc698d87 100644
> > --- a/t/unit-tests/t-reftable-stack.c
> > +++ b/t/unit-tests/t-reftable-stack.c
> > @@ -271,7 +271,7 @@ static void t_reftable_stack_transaction_api(void)
> >  
> >  	reftable_addition_destroy(add);
> >  
> > -	err = reftable_stack_new_addition(&add, st);
> > +	err = reftable_stack_new_addition(&add, st, 0);
> >  	check(!err);
> >  
> >  	err = reftable_addition_add(add, write_test_ref, &ref);
> > @@ -292,6 +292,68 @@ static void t_reftable_stack_transaction_api(void)
> >  	clear_dir(dir);
> >  }
> >  
> > +static void t_reftable_stack_transaction_with_reload(void)
> > +{
> > +	char *dir = get_tmp_dir(__LINE__);
> > +	struct reftable_stack *st1 = NULL, *st2 = NULL;
> > +	int err;
> > +	struct reftable_addition *add = NULL;
> > +	struct reftable_ref_record refs[2] = {
> > +		{
> > +			.refname = (char *) "refs/heads/a",
> > +			.update_index = 1,
> > +			.value_type = REFTABLE_REF_VAL1,
> > +			.value.val1 = { '1' },
> > +		},
> > +		{
> > +			.refname = (char *) "refs/heads/b",
> > +			.update_index = 2,
> > +			.value_type = REFTABLE_REF_VAL1,
> > +			.value.val1 = { '1' },
> > +		},
> > +	};
> > +	struct reftable_ref_record ref = { 0 };
> > +
> 
> Create two reftable stacks that provide a view into the reftable tables
> inside "dir".

Yup.

> > +	err = reftable_new_stack(&st1, dir, NULL);
> > +	check(!err);
> > +	err = reftable_new_stack(&st2, dir, NULL);
> > +	check(!err);
> > +
> 
> Successfully add refs[0] to the first stack using the transactional API.

Here we only open the stacks without doing anything with them yet. This
is preparation for being able to read/write them.

> > +	err = reftable_stack_new_addition(&add, st1, 0);
> > +	check(!err);
> > +	err = reftable_addition_add(add, write_test_ref, &refs[0]);
> > +	check(!err);
> > +	err = reftable_addition_commit(add);
> > +	check(!err);
> > +	reftable_addition_destroy(add);
> > +
> > +	/*
> > +	 * The second stack is now outdated, which we should notice. We do not
> > +	 * create the addition and lock the stack by default, but allow the
> > +	 * reload to happen when REFTABLE_STACK_NEW_ADDITION_RELOAD is set.
> > +	 */
> 
> We try to open a transaction via the second reftable stack, but the
> this stack is outdated because we've written to "dir" when the previous
> stack addition was committed.

Yup.

> > +	err = reftable_stack_new_addition(&add, st2, 0);
> > +	check_int(err, ==, REFTABLE_OUTDATED_ERROR);
> 
> Try again, but supply the flag so it performs a reload internally. Write
> refs[1] to "dir" by committing the transaction. 

Yup.

> > +	err = reftable_stack_new_addition(&add, st2, REFTABLE_STACK_NEW_ADDITION_RELOAD);
> > +	check(!err);
> > +	err = reftable_addition_add(add, write_test_ref, &refs[1]);
> > +	check(!err);
> > +	err = reftable_addition_commit(add);
> > +	check(!err);
> > +	reftable_addition_destroy(add);
> > +
> 
> Asserts.

Exactly.

Patrick