git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 00/14] ref-transactions-reflog

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Nov 20, 2014, 10:56 UTC
Message-ID
<546DC8E9.6090504@alum.mit.edu>
In-Reply-To
<CAGZ79kb3DOrL_txW-qxzd0=4sKrOiPTdSg-17_0+__wuj0TBaQ@mail.gmail.com>
On 11/20/2014 12:22 AM, Stefan Beller wrote:
Show 6 quoted lines
> Sorry for the long delay.
> Thanks for the explanation and discussion.
> 
> So do I understand it right, that you are not opposing
> the introduction of "everything should go through transactions"
> but rather the detail and abstraction level of the API?
Correct.
Show 10 quoted lines
> So starting from Michaels proposal in the first response:
> 
> 1. Add a reflog entry when a reference is updated in a transaction.
> 
> ok
> 
> 2. Rename a reflog file when the corresponding reference is renamed.
> 
> This should happen within the same transaction as the reference is
> renamed, right?

Yes. Maybe there should be a "rename reference" operation that can be added to a transaction, and it simply knows to rename any associated reflogs. Then the calling code wouldn't have to worry about reflogs explicitly in this case at all.

> So we don't have a multistep process here, which may abort in between having the
> reference updated and a broken reflog or vice versa. We want to either
> have both
> the ref and the reflog updated or neither.
Yes.
> 3. Delete the reflog when the corresponding reference is deleted [1].
> 
> also as one transaction?

It would be a side-effect of committing a transaction that contains a reference deletion. The deletion of the reflog would be done at the same time that the rest of the transaction is committed, and again the calling code wouldn't have to explicitly worry about the reflogs.

Show 13 quoted lines
> 4. Configure a reference to be reflogged.
> 5. Configure a reference to not be reflogged anymore and delete any
>    existing reflog.
> 
> Why do we need 4 and 5 here? Wouldn't all refs be reflog by default and
> why do I want to exclude some?
> 
> 6. Selectively expire old reflog entries, e.g., based on their age.
> 
> This is the maintenance operation, which you were talking about.
> In my vision, this also should go into one transaction. So you have the
> business logic figuring out all the changes ("drop reflog entry a b and d")
> and within one transaction we can perform all of the changes.

But if we take the approach described above, AFAICT this operation is the only one that would require the caller to manipulate reflog entries explicitly. And it has to iterate through the old reflog entries, decide which ones to keep, possibly change its neighbors to eliminate gaps in the chain, then stuff each of the reflog entries into a transaction one by one. To allow this to be implemented on the caller side, the transaction API has to be complicated in the following ways:

* Add a transaction_update_type (UPDATE_SHA1 vs. UPDATE_LOG).
* Add reflog_fd, reflog_lock, and committer members to struct ref_update.
* New function transaction_update_reflog().
* A new flag REFLOG_TRUNCATE that allows the reflog file to be truncated
before writing.
* Machinery that recognizes that a transaction contains multiple reflog
updates for the same reference and processes them specially to avoid
locking and rewriting the reflog file multiple times.

So this design has the caller serializing all reflog entries into separate ref_update structs (which implies that they are held in RAM!) only for ref_transaction_commit() to scan through all ref_updates looking for reflog updates that go together so that they can be processed as a whole. In other words, the caller picks the reflog apart and then ref_transaction_commit() glues it back together. It's all very contrived.

I suggest that the caller only be responsible for deciding which reflog entries to keep (by supplying a callback function), and a new expire_reflogs_for_me_please() API function be responsible for taking out a lock, feeding the old reflog entries to the callback, expiring the unwanted entries, optionally eliminating gaps in the chain (for the use of "reflog [expire|delete] --rewrite"), writing the new reflog entries, and optionally updating the reference itself (for the use of "reflog [expire|delete] --updateref").

The benefit will be simpler code, a better separation of responsibilities, and a simpler VTABLE that future reference backends have to implement.

I would love to work on this but unfortunately have way too much on my plate right now.

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
Previous: Junio C HamanoNext: Jonathan Nieder
Message 25 of 31 in “ref-transactions-reflog”
  1. 00/14 ref-transactions-reflogStefan Beller, Nov 18, 2014
  2. 01/14 refs.c: make ref_transaction_create a wrapper for ref_transaction_updateStefan Beller, Nov 18, 2014
  3. 02/14 refs.c: make ref_transaction_delete a wrapper for ref_transaction_updateStefan Beller, Nov 18, 2014
  4. 03/14 refs.c: rename the transaction functionsStefan Beller, Nov 18, 2014
  5. 04/14 refs.c: add a function to append a reflog entry to a fdStefan Beller, Nov 18, 2014
  6. 05/14 refs.c: add a new update_type field to ref_updateStefan Beller, Nov 18, 2014
  7. 06/14 refs.c: add a transaction function to append a reflog entryStefan Beller, Nov 18, 2014
  8. 07/14 refs.c: add a flag to allow reflog updates to truncate the logStefan Beller, Nov 18, 2014
  9. 08/14 refs.c: only write reflog update if msg is non-NULLStefan Beller, Nov 18, 2014
  10. 09/14 refs.c: allow multiple reflog updates during a single transactionStefan Beller, Nov 18, 2014
  11. 10/14 reflog.c: use a reflog transaction when writing during expireStefan Beller, Nov 18, 2014
  12. 11/14 refs.c: rename log_ref_setup to create_reflogStefan Beller, Nov 18, 2014
  13. 12/14 refs.c: Remove unlock_ref/close_ref/commit_ref from the refs apiStefan Beller, Nov 18, 2014
  14. 13/14 refs.c: remove lock_any_ref_for_updateStefan Beller, Nov 18, 2014
  15. 14/14 refs.c: allow deleting refs with a broken sha1Stefan Beller, Nov 18, 2014
  16. Michael HaggertyNov 18, 2014
  17. Ronnie SahlbergNov 18, 2014
  18. Michael HaggertyNov 18, 2014
  19. Junio C HamanoNov 18, 2014
  20. Michael HaggertyNov 18, 2014
  21. Junio C HamanoNov 18, 2014
  22. Stefan BellerNov 19, 2014
  23. Jonathan NiederNov 20, 2014
  24. Junio C HamanoNov 20, 2014
  25. Michael HaggertyNov 20, 2014
  26. Jonathan NiederNov 20, 2014
  27. 0/4 Using transactions for the reflogStefan Beller, Nov 27, 2014
  28. 1/4 refs.c: rename the transaction functionsStefan Beller, Nov 27, 2014
  29. 2/4 refs.c: add a new update_type field to ref_updateStefan Beller, Nov 27, 2014
  30. 3/4 refs.c: add a transaction function to append a reflog entryStefan Beller, Nov 27, 2014
  31. 4/4 reflog.c: use a reflog transaction when writing during expireStefan Beller, Nov 27, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.