Patrick Steinhardt <ps@xxxxxx> writes: > 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 Thanks. >> 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. > Which works, because for reflog migration, the caller _has_ to specify 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. > Exactly. Currently this is entirely and only used in the migration code. Perhaps we expose reflog addition to users, but that is not something being pursued. >> + /* >> + * 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 Thanks
Attachment:
signature.asc
Description: PGP signature
