Karthik Nayak <karthik.188@xxxxxxxxx> writes: > Subject: [PATCH] reftable: write correct max_update_index to header > > In 297c09eabb (refs: allow multiple reflog entries for the same refname, > 2024-12-16), the reftable backend learned to handle multiple reflog > entries within the same transaction. This was done modifying the "done" -> "done by", I think. > To fix the issue, the appropriate `max_update_index` limit must be set > even before the first block is written. Add a `max_index` field to the > transaction which holds the `max_index` within all its updates, then > propagate this value to the reftable backend, wherein this is used to > the set the `max_update_index` correctly. > diff --git a/refs.c b/refs.c > index 0f41b2fd4a..f7b6f0f897 100644 > --- a/refs.c > +++ b/refs.c > @@ -1345,6 +1345,13 @@ int ref_transaction_update_reflog(struct Not the focus of this topic/fix, but I notice that the only caller of this ref_transaction_update_reflog() function is a static function migrate_one_reflog_entry() in the same file. Do we expect that we would add more callers? Otherwise we should make it a file scope static and remove it from <refs.h> file. > ref_transaction *transaction, > update->flags &= ~REF_HAVE_OLD; > update->index = index; > > + /* > + * Reference backends may need to know the max index to optimize > + * their writes. So we store the max_index on the transaction level. > + */ > + if (index > transaction->max_index) > + transaction->max_index = index; > + > return 0; > } So from the problem description, whenever we consume an index by assigning it to an update that belongs to a transaction, we must make sure that transaction's max_index covers that value of the index? I was wondering if we should have a less error prone way to do that by having a helper function that takes ref_update and ref_transaction objects to do the above, but this is exclusively used by reflog migration code path and nowhere else, so it may probably be fine as-is. I dunno.
