Re: [PATCH 2/8] refs/reftable: handle reloading stacks in the reftable backend
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Nov 5, 2024, 11:14 UTC
- Message-ID
- <CAOLa=ZQwUGzjmvCv8TKuwbeePTG6+HNjU3ngO6hD8=xSOQ5kaQ@mail.gmail.com>
- In-Reply-To
- <b81ce63589e0380baf2b977a6abd706a66b08bee.1730732881.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 11 quoted lines
> @@ -898,27 +915,31 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out,
> struct ref_update *update,
> struct strbuf *err)
> {
> - struct reftable_stack *stack = backend_for(refs, update->refname, NULL)->stack;
> struct write_transaction_table_arg *arg = NULL;
> + struct reftable_backend *be;
> size_t i;
> int ret;
>
> + ret = backend_for(&be, refs, update->refname, NULL, 0);So here, we don't reload the stack, it would be nice to add a comment why, in the places we don't. Here, it seems to be because we possibly already have an update which would have pushed us to reload the stack.
> + if (ret) > + return ret; > +
[snip]
Show 15 quoted lines
> @@ -1995,15 +2022,19 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
> {
> struct reftable_ref_store *refs =
> reftable_be_downcast(ref_store, REF_STORE_READ, "for_each_reflog_ent_reverse");
> - struct reftable_stack *stack = backend_for(refs, refname, &refname)->stack;
> struct reftable_log_record log = {0};
> struct reftable_iterator it = {0};
> + struct reftable_backend *be;
> int ret;
>
> if (refs->err < 0)
> return refs->err;
>
> - ret = reftable_stack_init_log_iterator(stack, &it);
> + ret = backend_for(&be, refs, refname, &refname, 0);Like here, I'm not entirely sure why we don't reload the stack.
Show 7 quoted lines
> + if (ret) > + goto done; > + > + ret = reftable_stack_init_log_iterator(be->stack, &it); > if (ret < 0) > goto done; >
[snip]
Show 8 quoted lines
> @@ -2462,7 +2498,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, > arg.refs = refs; > arg.records = rewritten; > arg.len = logs_nr; > - arg.stack = stack, > + arg.stack = be->stack, > arg.refname = refname, >
Shouldn't these lines end with ';'?
> ret = reftable_addition_add(add, &write_reflog_expiry_table, &arg); > -- > 2.47.0.229.g8f8d6eee53.dirty