running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste

Linus Torvalds· May 22, 2005, 19:09 UTC · re: Thomas Glanzmann · lore

Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste

On Sun, 22 May 2005, Thomas Glanzmann wrote:

>
> I wonder why 'git-update-cache --refresh' running in the same directory
> shared via NFS ends up in reindexing the whole files when running on
> different machines on a NFS share.

It does?

Can you check what 

	ls -li --time=atime

shows on the different clients? Also, try "ctime".

> Is there a reason for this or can it easily be fixes. I also wonder if
> the locking which is used to lock the cache is 'nfs safe'.

		Linus

Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste

Hello,

> It does?

Not for me at the moment:

> Can you check what 

> 	ls -li --time=atime

> shows on the different clients? Also, try "ctime".

atime is different of course different.

@@ -1,11 +1,11 @@
 .:
 total 15
 5483033 -rw-r--r--  1 sithglan icipguru  391 May 22 21:14 Makefile
-1842682 drwxr-xr-x  2 sithglan icipguru  512 May 22 21:23 packages/
-5541351 drwxr-xr-x  2 sithglan icipguru  512 May 22 21:23 public_html/
-5541339 drwxr-xr-x  2 sithglan icipguru  512 May 22 21:23 scripts/
-5482949 drwxr-xr-x  2 sithglan icipguru 8704 May 22 21:23 sources/
-5482985 drwxr-xr-x  2 sithglan icipguru 2048 May 22 21:23 specs/
+1842682 drwxr-xr-x    2 sithglan icipguru      512 May 22 21:19 packages/
+5541351 drwxr-xr-x    2 sithglan icipguru      512 May 22 21:19 public_html/
+5541339 drwxr-xr-x    2 sithglan icipguru      512 May 22 21:19 scripts/
+5482949 drwxr-xr-x    2 sithglan icipguru     8704 May 22 21:19 sources/
+5482985 drwxr-xr-x    2 sithglan icipguru     2048 May 22 21:19 specs/

 ./packages:
 total 0

If you need the files:

http://wwwcip.informatik.uni-erlangen.de/~sithglan/faui01  (58k)
http://wwwcip.informatik.uni-erlangen.de/~sithglan/faui03  (61k)
http://wwwcip.informatik.uni-erlangen.de/~sithglan/faui04a (61k)

> It _should_ be safe. It does the old lockfile thing, with a "link()" that
> should guarantee atomicity. No fcntl locking or similar that can have
> problems with networked filesystems and different UNIXes.

Is link() NFS safe? I thought only mkdir() for nfs?

	Thomas

Linus Torvalds· May 22, 2005, 20:43 UTC · re: Thomas Glanzmann · lore

Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste

On Sun, 22 May 2005, Thomas Glanzmann wrote:

> 
> Is link() NFS safe? I thought only mkdir() for nfs?

		Linus

[PATCH] Don't include devicenumber into INODE_CHANGED test [WAS: Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste]

Hello,

> Sorry, I meant "rename", not "link", and yes, it should be NFS-safe. It's 
> how all the mailers do things too, afaik.

okay. I will doublecheck that and come back.

> As to your update-cache problem, it seems to be just due to NFS stat
> caching. You generally should _not_ work on two machines at the same time,
> but it probably does the right thing in the end.

#define INODE_CHANGED   0x0010

> In general, I would suggest using separate GIT repositories over sharing
> them over NFS. As far as I'm concerned, I think NFS should work in the
> sense that you can work from different clients at _different_times_, and
> I'm certainly not going to guarantee that two different clients that work
> at the same time against the same repository will get sane results.

> For example, if you do a "git-checkout-cache -f -a" at the same time, I 
> won't guarantee that things won't race on the working files. Don't do it.

	Thomas

diff --git a/update-cache.c b/update-cache.c
--- a/update-cache.c
+++ b/update-cache.c
@@ -174,6 +174,8 @@ static struct cache_entry *refresh_entry
 	if (!changed)
 		return ce;
 
+	fprintf(stderr, "name: %s 0x%08x\n", ce->name, changed);
+
 	/*
 	 * If the mode or type has changed, there's no point in trying
 	 * to refresh the entry - it's not going to match

Here is the real patch:

[PATCH] Don't include devicenumber into INODE_CHANGED test

This fixes the problem that git-update-cache --refresh rebuilds the
cache stat information everytime it is started on a different host while
working in the same NFS shared repository.

Signed-off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>

diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -65,8 +65,7 @@ int ce_match_stat(struct cache_entry *ce
 	if (ce->ce_uid != htonl(st->st_uid) ||
 	    ce->ce_gid != htonl(st->st_gid))
 		changed |= OWNER_CHANGED;
-	if (ce->ce_dev != htonl(st->st_dev) ||
-	    ce->ce_ino != htonl(st->st_ino))
+	if (ce->ce_ino != htonl(st->st_ino))
 		changed |= INODE_CHANGED;
 	if (ce->ce_size != htonl(st->st_size))
 		changed |= DATA_CHANGED;

Alternate Patch: [PATCH] Don't include device number in cache invalidation when running on NFS

Hello,

* Thomas Glanzmann <sithglan@stud.uni-erlangen.de> [050522 23:24]:

> Hello,

> > Sorry, I meant "rename", not "link", and yes, it should be NFS-safe. It's 
> > how all the mailers do things too, afaik.

> okay. I will doublecheck that and come back.

yes, you're right.

While reading liblockfile I saw the following:

/*
 *      See if the directory where is certain file is in
 *      is located on an NFS mounted volume.
 */
static int is_nfs(const char *file)
{
        char dir[1024];
        char *s;
        struct stat st;

        strncpy(dir, file, sizeof(dir));
        if ((s = strrchr(dir, '/')) != NULL)
                *s = 0;
        else
                strcpy(dir, ".");

        if (stat(dir, &st) < 0)
                return 0;

        return ((st.st_dev & 0xFF00) == 0);
}

[PATCH] Don't include device number in cache invalidation when running on NFS

Signed-off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>

diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -65,8 +65,11 @@ int ce_match_stat(struct cache_entry *ce
 	if (ce->ce_uid != htonl(st->st_uid) ||
 	    ce->ce_gid != htonl(st->st_gid))
 		changed |= OWNER_CHANGED;
-	if (ce->ce_dev != htonl(st->st_dev) ||
-	    ce->ce_ino != htonl(st->st_ino))
+	/* Only include device number if not running on NFS */
+	if (ce->ce_dev != htonl(st->st_dev) &&
+	    ((st->st_dev & 0xFF00) == 0))
+		changed |= INODE_CHANGED;
+	if (ce->ce_ino != htonl(st->st_ino))
 		changed |= INODE_CHANGED;
 	if (ce->ce_size != htonl(st->st_size))
 		changed |= DATA_CHANGED;

Linus Torvalds· May 22, 2005, 21:58 UTC · re: Thomas Glanzmann · lore

Re: Alternate Patch: [PATCH] Don't include device number in cache invalidation when running on NFS

On Sun, 22 May 2005, Thomas Glanzmann wrote:

> 
> While reading liblockfile I saw the following:

	if (!major(st.st_dev))

but even that is very Linux-specific.

> [PATCH] Don't include device number in cache invalidation when running on NFS

		Linus

Re: Alternate Patch: [PATCH] Don't include device number in cache invalidation when running on NFS

Hello,

> This is _really_ Linux-specific afaik. Which is ok for git, but at the
> same time it really makes me go "Ewww". It's testing that the major number 
> is 0, and it would be a lot more cleaner to use 

> 	if (!major(st.st_dev))

> but even that is very Linux-specific.

I see.

> I'll have to think about it. Maybe I should just remove the st_dev check. 
> I guess inode/size/mtime/ctime should be plenty safe enough in practice.

	Thomas