If two or more different users perform a git-fetch on the same mirror
(--mirror) repo concurrently, could that cause corruption? I tried a manual
test using the git protocol over separate machines and they both thought
they needed to do the full updates and they both appeared to work. I'm not
sure if git is serializing this, or if it is possible for concurrent fetches
to step on each other.
v/r,
Neal
From: Jeff King <hidden> Date: 2016-06-15 22:50:20
On Wed, Jan 05, 2011 at 02:33:36PM -0600, Neal Kreitzinger wrote:
If two or more different users perform a git-fetch on the same mirror
(--mirror) repo concurrently, could that cause corruption? I tried a manual
test using the git protocol over separate machines and they both thought
they needed to do the full updates and they both appeared to work. I'm not
sure if git is serializing this, or if it is possible for concurrent fetches
to step on each other.
No, it shouldn't cause corruption, but it will cause wasted effort and
it may cause one to report failure. The fetch process gets all of the
objects first, and then updates the ref (so we never have refs that
point to object we didn't get yet). So both of the concurrent fetches
will see that we have a big set of objects to get and will work on
getting them at the same time, after which they will update the refs
appropriately (presumably to the same thing).
I haven't looked specifically at how fetch does locking, but usually the
procedure is to lock the ref, fetch the old value, unlock it, then do
some long-running task (like fetching objects), then lock again, check
that the old value didn't change out from under us, update it, then
unlock. In which case one of the fetches might see "oops, somebody
updated while we were fetching" and complain.
However, in the default configuration, we fetch using a "+" refspec,
which forces update of the ref even in the case of a non-fast-forward. I
don't know whether that force also would override any lock-checking.
-Peff
On Wed, Jan 5, 2011 at 12:47, Jeff King [off-list ref] wrote:
However, in the default configuration, we fetch using a "+" refspec,
which forces update of the ref even in the case of a non-fast-forward. I
don't know whether that force also would override any lock-checking.
Nope, it doesn't. We still use locking to update the refs, to ensure
the update is seen atomically by a reader. The + just means don't
check that the old value is fully reachable from the new after the
lock as been taken.
If both fetch processes try to update the same ref at the same time,
one will get the lock and continue, and the other will crash with an
error (because the lock was busy). If one is slightly slower than the
other, they will probably update the refs twice, with the slower fetch
updating what the faster one had just updated. :-)
--
Shawn.
From: Jeff King <hidden> Date: 2016-06-15 22:50:20
On Wed, Jan 05, 2011 at 12:51:12PM -0800, Shawn Pearce wrote:
On Wed, Jan 5, 2011 at 12:47, Jeff King [off-list ref] wrote:
quoted
However, in the default configuration, we fetch using a "+" refspec,
which forces update of the ref even in the case of a non-fast-forward. I
don't know whether that force also would override any lock-checking.
Nope, it doesn't. We still use locking to update the refs, to ensure
the update is seen atomically by a reader. The + just means don't
check that the old value is fully reachable from the new after the
lock as been taken.
Good, that's what IMHO it _should_ do. :)
If both fetch processes try to update the same ref at the same time,
one will get the lock and continue, and the other will crash with an
error (because the lock was busy). If one is slightly slower than the
other, they will probably update the refs twice, with the slower fetch
updating what the faster one had just updated. :-)
I assumed it would take the "old" value at the very beginning of the
fetch (before talking with the remote), and then see that the ref was
changed under our feet. Or does it simply do it at the end?
... goes to read code ...
-Peff
From: Jeff King <hidden> Date: 2016-06-15 22:50:20
On Wed, Jan 05, 2011 at 03:53:25PM -0500, Jeff King wrote:
quoted
If both fetch processes try to update the same ref at the same time,
one will get the lock and continue, and the other will crash with an
error (because the lock was busy). If one is slightly slower than the
other, they will probably update the refs twice, with the slower fetch
updating what the faster one had just updated. :-)
I assumed it would take the "old" value at the very beginning of the
fetch (before talking with the remote), and then see that the ref was
changed under our feet. Or does it simply do it at the end?
Hmm. Weirder even, builtin/fetch.c:s_update_ref takes a "check_old"
flag, and we do always use it for branch updates. But not for tag
updates. I can't think of why. The code blames all the way back to the
original builtin-fetch.
Anyway, when we do check, we check the value from the beginning of the
fetch. So you can get lock conflicts. For example, doing this:
mkdir repo && cd repo && git init
echo contents >foo && git add . && git commit -m one
git update-ref refs/remotes/origin/master refs/heads/master
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read' & xterm -e 'git fetch -v; read'
I.e., putting some cruft into the ref and then updating it. One fetch
will force-write over the ref properly:
+ ac32203...4e64590 master -> origin/master (forced update)
but the other one will barf on the lock:
error: Ref refs/remotes/origin/master is at 4e6459052ab329914c7712a926773e566b8c821d but expected ac32203727daa3bcb5fc041786aa45adbbe86299
...
! ac32203...4e64590 master -> origin/master (unable to update local ref)
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo && cd repo && git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read' & xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
I wonder if we should, but perhaps there is some corner case I am not
considering. The code is in lock_ref_sha1_basic, but blaming didn't turn
up anything helpful.
-Peff
On Wed, Jan 05, 2011 at 03:53:25PM -0500, Jeff King wrote:
quoted
quoted
If both fetch processes try to update the same ref at the same time,
one will get the lock and continue, and the other will crash with an
error (because the lock was busy). If one is slightly slower than the
other, they will probably update the refs twice, with the slower fetch
updating what the faster one had just updated. :-)
I assumed it would take the "old" value at the very beginning of the
fetch (before talking with the remote), and then see that the ref was
changed under our feet. Or does it simply do it at the end?
Hmm. Weirder even, builtin/fetch.c:s_update_ref takes a "check_old"
flag, and we do always use it for branch updates. But not for tag
updates. I can't think of why. The code blames all the way back to the
original builtin-fetch.
Anyway, when we do check, we check the value from the beginning of the
fetch. So you can get lock conflicts. For example, doing this:
mkdir repo&& cd repo&& git init
echo contents>foo&& git add .&& git commit -m one
git update-ref refs/remotes/origin/master refs/heads/master
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read'& xterm -e 'git fetch -v; read'
I.e., putting some cruft into the ref and then updating it. One fetch
will force-write over the ref properly:
+ ac32203...4e64590 master -> origin/master (forced update)
but the other one will barf on the lock:
error: Ref refs/remotes/origin/master is at 4e6459052ab329914c7712a926773e566b8c821d but expected ac32203727daa3bcb5fc041786aa45adbbe86299
...
! ac32203...4e64590 master -> origin/master (unable to update local ref)
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo&& cd repo&& git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read'& xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
I wonder if we should, but perhaps there is some corner case I am not
considering. The code is in lock_ref_sha1_basic, but blaming didn't turn
up anything helpful.
-Peff
This was actually the case in my test. Updates to the mirror are always
new branches except for master. The only pre-existing branch that might
get updated is master, but in that test it didn't. The new branches and
tags were updated. The new tags always point to the new branches. I'm
running 1.7.1 on both servers.
v/r,
Neal
On Wed, Jan 05, 2011 at 03:53:25PM -0500, Jeff King wrote:
quoted
quoted
If both fetch processes try to update the same ref at the same time,
one will get the lock and continue, and the other will crash with an
error (because the lock was busy). If one is slightly slower than the
other, they will probably update the refs twice, with the slower fetch
updating what the faster one had just updated. :-)
I assumed it would take the "old" value at the very beginning of the
fetch (before talking with the remote), and then see that the ref was
changed under our feet. Or does it simply do it at the end?
Hmm. Weirder even, builtin/fetch.c:s_update_ref takes a "check_old"
flag, and we do always use it for branch updates. But not for tag
updates. I can't think of why. The code blames all the way back to the
original builtin-fetch.
Anyway, when we do check, we check the value from the beginning of the
fetch. So you can get lock conflicts. For example, doing this:
mkdir repo&& cd repo&& git init
echo contents>foo&& git add .&& git commit -m one
git update-ref refs/remotes/origin/master refs/heads/master
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read'& xterm -e 'git fetch -v; read'
I.e., putting some cruft into the ref and then updating it. One fetch
will force-write over the ref properly:
+ ac32203...4e64590 master -> origin/master (forced update)
but the other one will barf on the lock:
error: Ref refs/remotes/origin/master is at 4e6459052ab329914c7712a926773e566b8c821d but expected ac32203727daa3bcb5fc041786aa45adbbe86299
...
! ac32203...4e64590 master -> origin/master (unable to update local ref)
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo&& cd repo&& git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read'& xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
I wonder if we should, but perhaps there is some corner case I am not
considering. The code is in lock_ref_sha1_basic, but blaming didn't turn
up anything helpful.
-Peff
In the case of concurrent pulls to the same non-bare repo, could the
working tree or index get corrupted, or does git have concurrency
control mechanisms for this too?
v/r,
Neal
From: Jeff King <hidden> Date: 2016-06-15 22:50:20
On Wed, Jan 05, 2011 at 04:42:49PM -0600, Neal Kreitzinger wrote:
In the case of concurrent pulls to the same non-bare repo, could the
working tree or index get corrupted, or does git have concurrency
control mechanisms for this too?
There's a lock on the index, so it shouldn't be corruptable; one process
will just end up waiting. I'm not sure offhand whether writing working
tree files is done under any lock, but I would tend to think not, since
it can be a long process. However, writing the same file twice should be
OK; we unlink the old version and create the new from scratch. So the
first writer will get its write-in-progress unlinked, and the second one
will "win".
-Peff