On Fri, Dec 13, 2024 at 11:36:52AM +0100, Karthik Nayak wrote: > The reference transaction only allows a single update for a given > reference to avoid conflicts. This, however, isn't an issue for reflogs. > There are no conflicts to be resolved in reflogs and when migrating > reflogs between backends we'd have multiple reflog entries for the same > refname. > > So allow multiple reflog updates within a single transaction. Also the > reflog creation logic isn't exposed to the end user. While this might > change in the future, currently, this reduces the scope of issues to > think about. > > In the reftable backend, the writer sorts all updates based on the > update_index before writing to the block. When there are multiple > reflogs for a given refname, it is essential that the order of the > reflogs is maintained. So add the `index` value to the `update_index`. > The `index` field is only be set when multiple reflog entries for a s/only be/only > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c > index b2e3ba877de9e59fea5a4d066eb13e60ef22a32b..bec5962debea7b62572d08f6fa8fd38ab4cd8af6 100644 > --- a/refs/reftable-backend.c > +++ b/refs/reftable-backend.c > @@ -1405,7 +1407,19 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data > } > > fill_reftable_log_record(log, &c); > - log->update_index = ts; > + > + /* > + * Updates are sorted by the writer. So updates for the same > + * refname need to contain different update indices. > + */ > + log->update_index = ts + u->index; Okay. So instead of tracking things via a map, we now rely on the caller to provide the update index. And if they don't provide one then we cannot guarantee ordering. I guess that's a good solution. After all, there will only be a very limited amount of callers in the first place, so I think it's fine to shift the responsibility onto them to maintain reflog ordering. They're also the only ones who really know about the actual ordering. > + /* > + * Note the max update_index so the limit can be set later on. > + */ > + if (log->update_index > max_update_index) > + max_update_index = log->update_index; Makes sense, as well. Patrick
