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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:48

Junio C Hamano [off-list ref] writes:
Ronnie Sahlberg [off-list ref] writes:
quoted
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 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?

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

From: Ronnie Sahlberg <hidden>
Date: 2016-06-15 23:02:48

On Tue, Oct 28, 2014 at 12:56 PM, Junio C Hamano [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
Ronnie Sahlberg [off-list ref] writes:
quoted
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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help