what's the current wisdom on git over NFS/CIFS?

Hi all,

A couple of us were beating each other senseless on this
issue over on #git, so I thought I'd ask: is it OK to do
this?  Or would there potential be race conditions due to
the lack of proper locking?

The git faq says SMBFS is not recommended.  Searching for
"git on NFS" gets me too many hits about the NFS _code_ :-)

Thanks,

Sitaram

On Thu, Jul 2, 2009 at 3:11 PM, Sitaram Chamarty[off-list ref] wrote:

The git faq says SMBFS is not recommended.  Searching for
"git on NFS" gets me too many hits about the NFS _code_ :-)

You can use marc.theaimsgroup.info to search the git list archives.

hth



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

On Thu, Jul 2, 2009 at 3:58 PM, Martin
Langhoff[off-list ref] wrote:

You can use marc.theaimsgroup.info to search the git list archives.

Actually, marc.theaimsgroup.com - which redirects to marc.info . See

   http://marc.info/?l=git&w=2&r=1&s=nfs&q=b

hth,


m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

On Thu, 2 Jul 2009, Sitaram Chamarty wrote:

A couple of us were beating each other senseless on this
issue over on #git, so I thought I'd ask: is it OK to do
this?  Or would there potential be race conditions due to
the lack of proper locking?

It should all work fine.

We've had a few NFS issues due to oddities with renaming across 
directories, but modern git avoids the cross-directory renames, and that 
whole issue only hit some very specific cases anyway.

And git doesn't have "proper locking", because it doesn't need it for 
database ops: git objects are stable. For refs, git should be using the 
proper NFS-safe "create and atomic rename" ops.

You do need to be a bit careful if you do maintenance operations 
concurrently (I would suggest avoiding doing concurrent "git gc --prune", 
for example), but any normal git workflow should be fine.

There is one big thing to look out for: performance. You may want to add

	[core]
		PreloadIndex = true

in your .git/config file, especially if you have a high-latency NFS server 
(but if you have a fast network and a high-end NFS server, you might be 
better off without it, so do your own testing).

Btw, I think we fixed the problem we had with CIFS. That one was a cifs 
filesystem problem on Linux, but it should be fixed in 2.6.30+ (commit 
0f4d634c: "cifs: flush data on any setattr"). If you have an older kernel 
(or are just uncertain), you can also work around it with

	[core]
		fsyncobjectfiles = true

which may be a good thing in general (regardless of any cifs issues), but 
in most cases the performance loss isn't worth it if your filesystem is 
stable and sane.

			Linus

On Fri, Jul 3, 2009 at 2:28 AM, Linus
Torvalds[off-list ref] wrote:

On Thu, 2 Jul 2009, Sitaram Chamarty wrote:

quoted

A couple of us were beating each other senseless on this
issue over on #git, so I thought I'd ask: is it OK to do
this?  Or would there potential be race conditions due to
the lack of proper locking?

It should all work fine.

We've had a few NFS issues due to oddities with renaming across
directories, but modern git avoids the cross-directory renames, and that
whole issue only hit some very specific cases anyway.

And git doesn't have "proper locking", because it doesn't need it for
database ops: git objects are stable. For refs, git should be using the
proper NFS-safe "create and atomic rename" ops.

You do need to be a bit careful if you do maintenance operations
concurrently (I would suggest avoiding doing concurrent "git gc --prune",
for example), but any normal git workflow should be fine.

There is one big thing to look out for: performance. You may want to add

       [core]
               PreloadIndex = true

Great!  Thanks for the really detailed reply; appreciate it very much.

I knew about core.PreloadIndex simply due to searching "man
git-config" for NFS etc., but not the rest of it.

in your .git/config file, especially if you have a high-latency NFS server
(but if you have a fast network and a high-end NFS server, you might be
better off without it, so do your own testing).

Btw, I think we fixed the problem we had with CIFS. That one was a cifs
filesystem problem on Linux, but it should be fixed in 2.6.30+ (commit
0f4d634c: "cifs: flush data on any setattr"). If you have an older kernel
(or are just uncertain), you can also work around it with

       [core]
               fsyncobjectfiles = true

which may be a good thing in general (regardless of any cifs issues), but
in most cases the performance loss isn't worth it if your filesystem is
stable and sane.

Though I asked this following from a debate on IRC, it now looks as if
this will solve another of my problems too.

Let me explain.

I'm evangelising git at work, and although most projects are happy,
even eager to setup a proper server for git, and those that can't are
happy to just use mine, there are a couple of projects that are almost
exclusively Windows _and_ cannot add another machine _and_ have client
confidentiality issues so they can't just use my server.

My alternatives for them so far were (1) VirtualBox running Fedora or
something within one of their beefier Windows servers or (2) the whole
cygwin install, which is painful compared to msysgit.

Sounds like we can just do it with traditional Windows fileshares, as
long as we make sure no one does a "git gc" on the bare repo that is
being shared.  That's a very small price to pay!

Thanks once again, and I'm copying Gitte in case he has anything to
add, seeing I have sneakily changed the focus of the question now :-)
[It wasn't pre-meditated, honest!]

Sitaram

Sitaram Chamarty schrieb:

On Fri, Jul 3, 2009 at 2:28 AM, Linus
Torvalds[off-list ref] wrote:

quoted

Btw, I think we fixed the problem we had with CIFS. That one was a cifs
filesystem problem on Linux, but it should be fixed in 2.6.30+ (commit
0f4d634c: "cifs: flush data on any setattr"). If you have an older kernel
(or are just uncertain), you can also work around it with

       [core]
               fsyncobjectfiles = true

which may be a good thing in general (regardless of any cifs issues), but
in most cases the performance loss isn't worth it if your filesystem is
stable and sane.

Though I asked this following from a debate on IRC, it now looks as if
this will solve another of my problems too.

Let me explain.

I'm evangelising git at work, and although most projects are happy,
even eager to setup a proper server for git, and those that can't are
happy to just use mine, there are a couple of projects that are almost
exclusively Windows _and_ cannot add another machine _and_ have client
confidentiality issues so they can't just use my server.

I don't exactly understand why your reply with this Windows story to the
fsyncobjectfiles paragraph. But if you think that fsyncobjectfiles+msysgit
is your killer argument, then you think wrong: fsyncobjectfiles is ignored
in msysgit. (We don't yet have an implementation of fsync() :-( )

The issue that Linus mentioned happened when a Linux client operates in a
repository that lives on a CIFS mount.

My alternatives for them so far were (1) VirtualBox running Fedora or
something within one of their beefier Windows servers or (2) the whole
cygwin install, which is painful compared to msysgit.

Sounds like we can just do it with traditional Windows fileshares, as
long as we make sure no one does a "git gc" on the bare repo that is
being shared.  That's a very small price to pay!

It will work with the caveat you mention. I have such a setup myself.

-- Hannes

On Fri, Jul 3, 2009 at 11:44 AM, Johannes Sixt[off-list ref] wrote:

Sitaram Chamarty schrieb:

quoted

On Fri, Jul 3, 2009 at 2:28 AM, Linus
Torvalds[off-list ref] wrote:

[snipped all of Linus' email]

quoted

Though I asked this following from a debate on IRC, it now looks as if
this will solve another of my problems too.

[snipped my "Windows story" ;-)]

I don't exactly understand why your reply with this Windows story to the
fsyncobjectfiles paragraph. But if you think that fsyncobjectfiles+msysgit

Not at all.  Since I was adding Gitte to the CC, I kept _all_ of
Linus' email quoted to provide him the full context.  My Windows story
pertains in general to the whole email, really.

quoted

Sounds like we can just do it with traditional Windows fileshares, as
long as we make sure no one does a "git gc" on the bare repo that is
being shared.  That's a very small price to pay!

It will work with the caveat you mention. I have such a setup myself.

And _that_ is what I was hoping to hear!  Thank you!  I can use it and
test it all I like, but hearing the guys who know say "it will work"
makes a big difference.

On Fri, Jul 03, 2009 at 08:14:46AM +0200, Johannes Sixt wrote:

(We don't yet have an implementation of fsync() :-( )

Why? It appears rather straightforward to me. Here is a patch that
implements it. Or did I miss something?

Warning: I do not use MinGW/Git, so I have not tested this patch.

-- >8 --
From 0a5e712ff8775e0f27923bedcb3c234288592eaa Mon Sep 17 00:00:00 2001
From: Dmitry Potapov <redacted>
Date: Fri, 3 Jul 2009 12:45:30 +0400
Subject: [PATCH] mingw: fsync implementation for Windows

Signed-off-by: Dmitry Potapov <redacted>
---
 compat/mingw.c |   16 ++++++++++++++++
 compat/mingw.h |    3 +--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index bed4178..65c9e8e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c

@@ -1231,3 +1231,19 @@ struct dirent *mingw_readdir(DIR *dir)
 	return (struct dirent*)&dir->dd_dir;
 }
 #endif // !NO_MINGW_REPLACE_READDIR
+
+int fsync(int fd)
+{
+	HANDLE h = (HANDLE) _get_osfhandle(fd);
+	if (h == INVALID_HANDLE_VALUE)
+	{
+		errno = EBADF;
+		return -1;
+	}
+	if (!FlushFileBuffers(h))
+	{
+		errno = err_win_to_posix(GetLastError());
+		return -1;
+	}
+	return 0;
+}

diff --git a/compat/mingw.h b/compat/mingw.h
index 4f7ba4c..14d53c0 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h

@@ -75,8 +75,7 @@ static inline int fork(void)
 { errno = ENOSYS; return -1; }
 static inline unsigned int alarm(unsigned int seconds)
 { return 0; }
-static inline int fsync(int fd)
-{ return 0; }
+int fsync(int fd);
 static inline int getppid(void)
 { return 1; }
 static inline void sync(void)

-- 
1.6.2.3

-- >8 --

Dmitry Potapov schrieb:

On Fri, Jul 03, 2009 at 08:14:46AM +0200, Johannes Sixt wrote:

quoted

(We don't yet have an implementation of fsync() :-( )

Why?

Nobody cared. ;)

It appears rather straightforward to me. Here is a patch that
implements it. Or did I miss something?

Yes, but it's not critical; see below ;)

+int fsync(int fd)
+{
+	HANDLE h = (HANDLE) _get_osfhandle(fd);
+	if (h == INVALID_HANDLE_VALUE)
+	{
+		errno = EBADF;
+		return -1;
+	}
+	if (!FlushFileBuffers(h))
+	{
+		errno = err_win_to_posix(GetLastError());
+		return -1;
+	}

Do we ever call fsync on a pipe? In this case, fsync should fail with
EINVAL, but your implementation would wait until the reader has drained
all data.

-static inline int fsync(int fd)
-{ return 0; }
+int fsync(int fd);

This declaration is now in the wrong section of mingw.h. And someone who
cares should test it first; so I won't bless this patch as is.

Thanks anyway,
-- Hannes

On Fri, 3 Jul 2009, Johannes Sixt wrote:

Do we ever call fsync on a pipe? In this case, fsync should fail with
EINVAL, but your implementation would wait until the reader has drained
all data.

I'm pretty sure we never call fsync() on anything but a file. So I suspect 
the patch for windows is fine - it only needs to care about regular files.

That said, there's a reason we don't enable fsync by default - it's 
generally not needed. I'm not sure what the NTFS crash semantics are, but 
I _think_ NTFS does everything with a proper log, and then fsync probably 
doesn't matter on windows.

The "fsync on CIFS" was not about "windows is crap, and because it's a 
windows filesystem we need to fsync" - it was literally because there was 
a bug in the Linux CIFS implementation that meant that pending writes 
could be lost if the file was made read-only while they were still 
pending.

And on non-journaling filesystems, fsync is obviously a good idea, but 
even then the state is often recoverable even in the face of a crash. Now, 
without fsync, you might have to work at it a bit and know what to do - 
like _manually_ resetting to the previous state and re-doing the last 
corrupt commit, throwing out any corrupt objects etc.

So enabling fsync can make things _much_ easier for recovery (to the point 
of making it possible at all for somebody who doesn't know the git object 
model), but is generally not a hard requirement.

Giving windows git the capability to do it sounds like a good idea, 
though.

Keep in mind: enabling fsync _will_ make object creation slower. Most 
people probably won't care much, though. It really only matters under some 
fairly rare circumstances - I care, beause it matters for things like 
"apply 200 patches in one go as fast as you can", but for most 'normal' 
workflows you'd probably never even notice.

			Linus