Patrick Steinhardt [off-list ref] writes:

On Fri, Dec 13, 2024 at 11:36:52AM +0100, Karthik Nayak wrote:

quoted

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.

quoted

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.

quoted

+				/*
+				 * 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