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

Re: [PATCH 05/15] refs.c: update rename_ref to use a transaction

From
Ronnie Sahlberg <sahlberg@google.com>
Date
Oct 28, 2014, 20:56 UTC
Message-ID
<CAL=YDWm05PyO07HbiOTiweh+3AEvXnbptbzoreLw-b9YUrm-Hg@mail.gmail.com>
In-Reply-To
<xmqqlho0j7dq.fsf@gitster.dls.corp.google.com>
On Tue, Oct 28, 2014 at 12:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Ronnie Sahlberg <sahlberg@google.com> writes:
>>
>>> commit 0295e9cebc41020ee84da275549b164a8770ffba upstream.
>>>
>>> Change refs.c to use a single transaction to copy/rename both the refs and
>>> its reflog. Since we are no longer using rename() to move the reflog file
>>> we no longer need to disallow rename_ref for refs with a symlink for its
>>> reflog so we can remove that test from the testsuite.
>>
>> Do you mean that we used to do a single rename(2) to move the entire
>> logfile, but now you copy potentially thousands of reflog entries
>> one by one?
>>
>> Hmmmm,... is that an improvement?

I think so. It makes to code a lot simpler and more atomic. As a side effect it removes restrictions for symlink handling and eliminates the two renames colliding race. Though, a read and then rewrite thousands of reflog entries will be slower than a single rename() syscall.

Show 6 quoted lines
>
> I see some value in "keep the original while creating a new one,
> just in case we fail to fully recreate the new one so that we can
> roll back with less programming effort".  But still, we should be
> able to copy the original to new without parsing and reformatting
> each and every entry, no?
Is renaming a branch with a long history is such a frequent or time
critical event?
I timed a git branch -m for a branch with ~2400 log entries and it
takes neglible time :
  real 0m0.008s
  user 0m0.000s
  sys 0m0.007s

During the special rename case, we are deleting one ref and creating another. For cases such as m->m/m or the reverse we must delete the old file/directory before we can create the new one.

The old rename code did this by renaming the file out to a common directory and then back to the new location. Which is fast (but a bit ...) The alternative is to read the old file into memory, delete it and then write the content back to the new location, which is kind of what the new code does.

If this turns out to be a bottleneck we can change the io when writing the reflog entries to use fwrite(). Lets see if there is a problem first.

regards ronnie sahlberg

Previous: Junio C HamanoNext: Junio C Hamano
Message 12 of 27 in “ref-transaction-rename”
  1. 00/15 ref-transaction-renameRonnie Sahlberg, Oct 21, 2014
  2. 01/15 refs.c: allow passing raw git_committer_info as email to _update_reflogRonnie Sahlberg, Oct 21, 2014
  3. 02/15 refs.c: return error instead of dying when locking fails during transactionRonnie Sahlberg, Oct 21, 2014
  4. Jeff KingNov 11, 2014
  5. Ronnie SahlbergNov 11, 2014
  6. 03/15 refs.c: use packed refs when deleting refs during a transactionRonnie Sahlberg, Oct 21, 2014
  7. Junio C HamanoOct 22, 2014
  8. 04/15 refs.c: use a stringlist for repack_without_refsRonnie Sahlberg, Oct 21, 2014
  9. 05/15 refs.c: update rename_ref to use a transactionRonnie Sahlberg, Oct 21, 2014
  10. Junio C HamanoOct 28, 2014
  11. Junio C HamanoOct 28, 2014
  12. Ronnie SahlbergOct 28, 2014
  13. Junio C HamanoOct 28, 2014
  14. Ronnie SahlbergOct 29, 2014
  15. Junio C HamanoOct 29, 2014
  16. Ronnie SahlbergOct 30, 2014
  17. 06/15 refs.c: rollback the lockfile before we die() in repack_without_refsRonnie Sahlberg, Oct 21, 2014
  18. 07/15 refs.c: move reflog updates into its own functionRonnie Sahlberg, Oct 21, 2014
  19. 08/15 refs.c: write updates to packed refs when a transaction has more than one refRonnie Sahlberg, Oct 21, 2014
  20. 09/15 remote.c: use a transaction for deleting refsRonnie Sahlberg, Oct 21, 2014
  21. 10/15 refs.c: make repack_without_refs staticRonnie Sahlberg, Oct 21, 2014
  22. 11/15 refs.c: make the *_packed_refs functions staticRonnie Sahlberg, Oct 21, 2014
  23. 12/15 refs.c: replace the onerr argument in update_ref with a strbuf errRonnie Sahlberg, Oct 21, 2014
  24. 13/15 refs.c: make add_packed_ref return an error instead of calling dieRonnie Sahlberg, Oct 21, 2014
  25. 14/15 refs.c: make lock_packed_refs take an err argumentRonnie Sahlberg, Oct 21, 2014
  26. 15/15 refs.c: add an err argument to pack_refsRonnie Sahlberg, Oct 21, 2014
  27. Junio C HamanoOct 30, 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.