{"thread":{"id":"14167","subject":"pread() over NFS (again) [1.5.5.4]","startedAt":"2008-06-26T16:40:27Z","lastAt":"2008-06-30T19:09:23Z","messageCount":16,"participants":["Christian Holtje","Shawn O. Pearce","Junio C Hamano","logank@sent.com","J. Bruce Fields","Trond Myklebust","Nicolas Pitre"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"81301","messageId":"6F25C1B4-85DE-4559-9471-BCD453FEB174@gmail.com","threadId":"14167","inReplyTo":null,"subject":"pread() over NFS (again) [1.5.5.4]","fromName":"Christian Holtje","fromEmail":"docwhat@gmail.com","sentAt":"2008-06-26T16:40:27Z","receivedAt":"2008-06-26T16:40:27Z","isPatch":false,"sender":{"key":"docwhat@gmail.com","avatar":"https://gravatar.com/avatar/b87944ebf1ff49feaeefaa1c19a6e98baee691c531ecb860c2bc4f63b464a395?d=mp&s=160"},"body":"I have read all the threads on git having trouble with pread() and I \ndidn't see anything to help.\n\nSituation:\n git commands run on system \"dev2\" which is Linux 2.6.9-42.0.8.ELsmp \non an NFS mounted directory.\n The NFS server is \"dev1\" which is Linux 2.4.28\n\nI get the following output from git when doing a fetch:\n remote: Counting objects: 406, done.\n remote: Compressing objects: 100% (198/198), done.\n remote: Total 253 (delta 127), reused 150 (delta 55)\n Receiving objects: 100% (253/253), 5.27 MiB | 9136 KiB/s, done.\n fatal: cannot pread pack file: No such file or directory\n fatal: index-pack failed\n\nThe end of the strace looks like so:\npread(3, \"\", 205, 1373) = 0\nwrite(2, \"fatal: cannot pread pack file: N\"..., 57) = 57\n\nI have ran strace -o /somedir/q -ff git fetch and have the straces \navailable at:\nhttp://docwhat.gerf.org/files/tmp/git-strace-20080626.tgz\n\nI have worked around the problem by running the fetch from the nfs \nserver.\n\nIt looks like I can recreate this at will and I'm willing to help \nfigure this out.\n\nThanks!\n\nCiao!\n"},{"id":"81322","messageId":"20080626204606.GX11793@spearce.org","threadId":"14167","inReplyTo":"6F25C1B4-85DE-4559-9471-BCD453FEB174@gmail.com","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-26T20:46:06Z","receivedAt":"2008-06-26T20:46:06Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Christian Holtje wrote:\n> I have read all the threads on git having trouble with pread() and I \n> didn't see anything to help.\n...\n> Receiving objects: 100% (253/253), 5.27 MiB | 9136 KiB/s, done.\n> fatal: cannot pread pack file: No such file or directory\n> fatal: index-pack failed\n> \n> The end of the strace looks like so:\n> pread(3, \"\", 205, 1373) = 0\n> write(2, \"fatal: cannot pread pack file: N\"..., 57) = 57\n\nHmmph. So pread for a length of 205 can return 0 on NFS? Is this\na transient error? If so, perhaps a patch like this might help:\n\ndiff --git a/index-pack.c b/index-pack.c\nindex 5ac91ba..737f757 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -309,14 +309,19 @@ static void *get_data_from_pack(struct object_entry *obj)\n \tunsigned char *src, *data;\n \tz_stream stream;\n \tint st;\n+\tint attempts = 0;\n \n \tsrc = xmalloc(len);\n \tdata = src;\n \tdo {\n \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n-\t\tif (n <= 0)\n+\t\tif (n <= 0) {\n+\t\t\tif (n == 0 && ++attempts < 10)\n+\t\t\t\tcontinue;\n \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n+\t\t}\n \t\trdy += n;\n+\t\tattempts = 0;\n \t} while (rdy < len);\n \tdata = xmalloc(obj->size);\n \tmemset(&stream, 0, sizeof(stream));\n\n\nThe file shouldn't be short unless someone truncated it, or there\nis a bug in index-pack. Neither is very likely, but I don't think\nwe would want to retry pread'ing the same block forever.\n\n-- \nShawn.\n"},{"id":"81324","messageId":"7vskuzq5ix.fsf@gitster.siamese.dyndns.org","threadId":"14167","inReplyTo":"20080626204606.GX11793@spearce.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-26T20:56:38Z","receivedAt":"2008-06-26T20:56:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Christian Holtje wrote:\n>> I have read all the threads on git having trouble with pread() and I \n>> didn't see anything to help.\n> ...\n>> Receiving objects: 100% (253/253), 5.27 MiB | 9136 KiB/s, done.\n>> fatal: cannot pread pack file: No such file or directory\n>> fatal: index-pack failed\n>> \n>> The end of the strace looks like so:\n>> pread(3, \"\", 205, 1373) = 0\n>> write(2, \"fatal: cannot pread pack file: N\"..., 57) = 57\n>\n> Hmmph. So pread for a length of 205 can return 0 on NFS? Is this\n> a transient error? If so, perhaps a patch like this might help:\n>\n> diff --git a/index-pack.c b/index-pack.c\n> index 5ac91ba..737f757 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -309,14 +309,19 @@ static void *get_data_from_pack(struct object_entry *obj)\n> \tunsigned char *src, *data;\n> \tz_stream stream;\n> \tint st;\n> +\tint attempts = 0;\n> \n> \tsrc = xmalloc(len);\n> \tdata = src;\n> \tdo {\n> \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> -\t\tif (n <= 0)\n> +\t\tif (n <= 0) {\n> +\t\t\tif (n == 0 && ++attempts < 10)\n> +\t\t\t\tcontinue;\n> \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> +\t\t}\n> \t\trdy += n;\n> +\t\tattempts = 0;\n> \t} while (rdy < len);\n> \tdata = xmalloc(obj->size);\n> \tmemset(&stream, 0, sizeof(stream));\n>\n>\n> The file shouldn't be short unless someone truncated it, or there\n> is a bug in index-pack. Neither is very likely, but I don't think\n> we would want to retry pread'ing the same block forever.\n\nI don't think we would want to retry even once. Return value of 0 from\npread is defined to be an EOF, isn't it?\n"},{"id":"81326","messageId":"20080626210556.GZ11793@spearce.org","threadId":"14167","inReplyTo":"7vskuzq5ix.fsf@gitster.siamese.dyndns.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-26T21:05:56Z","receivedAt":"2008-06-26T21:05:56Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> > Christian Holtje wrote:\n> >> I have read all the threads on git having trouble with pread() and I \n> >> didn't see anything to help.\n> > ...\n> >> Receiving objects: 100% (253/253), 5.27 MiB | 9136 KiB/s, done.\n> >> fatal: cannot pread pack file: No such file or directory\n> >> fatal: index-pack failed\n> >> \n> >> The end of the strace looks like so:\n> >> pread(3, \"\", 205, 1373) = 0\n> >> write(2, \"fatal: cannot pread pack file: N\"..., 57) = 57\n> >\n> > Hmmph. So pread for a length of 205 can return 0 on NFS? Is this\n> > a transient error? If so, perhaps a patch like this might help:\n...\n> > The file shouldn't be short unless someone truncated it, or there\n> > is a bug in index-pack. Neither is very likely, but I don't think\n> > we would want to retry pread'ing the same block forever.\n> \n> I don't think we would want to retry even once. Return value of 0 from\n> pread is defined to be an EOF, isn't it?\n\nIndeed, it is defined to be EOF, but EOF here makes no sense.\n\nWe have a file position we saw once before as the start of a delta.\nWe wrote it down to disk. We want to go back and open it up, as\nwe have the base decompressed and in memory and need to compute\nthe SHA-1 of the object that resides at this offset.\n\nAnd *wham* we get an EOF. Where there should be data. Where we\nknow there is data.\n\nI'm open to the idea that index-pack has a bug, but I doubt it.\nWe shovel hundreds of megabytes through that on a daily basis\nacross all of the git users, and nobody ever sees it crash out\nwith an EOF in the middle of an object it knows to be present.\nExcept poor Christian on NFS.\n\nActually, I think the last time someone reported something like this\nin Git it turned out to be an NFS kernel bug. I didn't quote it\nin my reply to him, but I think he did say this was a linux client,\nlinux server.\n\n-- \nShawn.\n"},{"id":"81328","messageId":"20AEFB67-2BE7-470F-B0EA-BDAC4B6DE576@gmail.com","threadId":"14167","inReplyTo":"20080626210556.GZ11793@spearce.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Christian Holtje","fromEmail":"docwhat@gmail.com","sentAt":"2008-06-26T21:36:08Z","receivedAt":"2008-06-26T21:36:08Z","isPatch":false,"sender":{"key":"docwhat@gmail.com","avatar":"https://gravatar.com/avatar/b87944ebf1ff49feaeefaa1c19a6e98baee691c531ecb860c2bc4f63b464a395?d=mp&s=160"},"body":"On Jun 26, 2008, at 5:05 PM, Shawn O. Pearce wrote:\n> Junio C Hamano wrote:\n>> \"Shawn O. Pearce\" writes:\n>>> Christian Holtje wrote:\n>>>> I have read all the threads on git having trouble with pread() \n>>>> and I\n>>>> didn't see anything to help.\n>>> ...\n>>>> Receiving objects: 100% (253/253), 5.27 MiB | 9136 KiB/s, done.\n>>>> fatal: cannot pread pack file: No such file or directory\n>>>> fatal: index-pack failed\n>>>>\n>>>> The end of the strace looks like so:\n>>>> pread(3, \"\", 205, 1373) = 0\n>>>> write(2, \"fatal: cannot pread pack file: N\"..., 57) = 57\n>>>\n>>> Hmmph. So pread for a length of 205 can return 0 on NFS? Is this\n>>> a transient error? If so, perhaps a patch like this might help:\n> ...\n>>> The file shouldn't be short unless someone truncated it, or there\n>>> is a bug in index-pack. Neither is very likely, but I don't think\n>>> we would want to retry pread'ing the same block forever.\n>>\n>> I don't think we would want to retry even once. Return value of 0 \n>> from\n>> pread is defined to be an EOF, isn't it?\n>\n> Indeed, it is defined to be EOF, but EOF here makes no sense.\n>\n> We have a file position we saw once before as the start of a delta.\n> We wrote it down to disk. We want to go back and open it up, as\n> we have the base decompressed and in memory and need to compute\n> the SHA-1 of the object that resides at this offset.\n>\n> And *wham* we get an EOF. Where there should be data. Where we\n> know there is data.\n>\n> I'm open to the idea that index-pack has a bug, but I doubt it.\n> We shovel hundreds of megabytes through that on a daily basis\n> across all of the git users, and nobody ever sees it crash out\n> with an EOF in the middle of an object it knows to be present.\n> Except poor Christian on NFS.\n>\n> Actually, I think the last time someone reported something like this\n> in Git it turned out to be an NFS kernel bug. I didn't quote it\n> in my reply to him, but I think he did say this was a linux client,\n> linux server.\n\nI did see the threads that talked about NFS, but I couldn't find any \nmatching messages in the linux kernel mailing list. If someone can \ngive me a pointer to where this picked up on other mailing lists (say, \nvia a URL) then I'll take that back to IT as a reason to upgrade.\n\nWould it be a bug in the client or server? I'd assume client...\n\nThe other NFS/pread() email thread was also a client of linux 2.6.9....\n\nCiao!\n"},{"id":"81329","messageId":"7vod5nq2dy.fsf@gitster.siamese.dyndns.org","threadId":"14167","inReplyTo":"20080626210556.GZ11793@spearce.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-26T22:04:25Z","receivedAt":"2008-06-26T22:04:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> We have a file position we saw once before as the start of a delta.\n> We wrote it down to disk. We want to go back and open it up, as\n> we have the base decompressed and in memory and need to compute\n> the SHA-1 of the object that resides at this offset.\n>\n> And *wham* we get an EOF. Where there should be data. Where we\n> know there is data.\n\nWe have written that earlier in the same process? Are we playing games\nwith mixed mmap() and pread()? Is fsync() or msync() or unmap/remap\nneeded?\n\n> Actually, I think the last time someone reported something like this\n> in Git it turned out to be an NFS kernel bug. I didn't quote it\n> in my reply to him, but I think he did say this was a linux client,\n> linux server.\n\nThis is getting into the area Linus would immediately know the answer to,\nbut he is away for this week.\n"},{"id":"81330","messageId":"20080626220755.GB11793@spearce.org","threadId":"14167","inReplyTo":"7vod5nq2dy.fsf@gitster.siamese.dyndns.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-26T22:07:55Z","receivedAt":"2008-06-26T22:07:55Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> \n> > We have a file position we saw once before as the start of a delta.\n> > We wrote it down to disk. We want to go back and open it up, as\n> > we have the base decompressed and in memory and need to compute\n> > the SHA-1 of the object that resides at this offset.\n> >\n> > And *wham* we get an EOF. Where there should be data. Where we\n> > know there is data.\n> \n> We have written that earlier in the same process? Are we playing games\n> with mixed mmap() and pread()? Is fsync() or msync() or unmap/remap\n> needed?\n\nYes, we wrote the very same file earlier in the process.\n\nindex-pack _used_ to use a mixed write/mmap game. Today it uses\nwrite, followed by later pread. No mmap. We had major performance\nissues on Mac OS X with mmap, not to mention the coherency issues\nthat can arise from using it. So index-pack is mmap free[*1*].\n \n> > Actually, I think the last time someone reported something like this\n> > in Git it turned out to be an NFS kernel bug. I didn't quote it\n> > in my reply to him, but I think he did say this was a linux client,\n> > linux server.\n> \n> This is getting into the area Linus would immediately know the answer to,\n> but he is away for this week.\n\nYea, I was expecting a reply from him, but that explains it.\n\n*1* There is usage of mmap in index-pack, but only to access\n _existing_ objects and packs from the object store.\n\n-- \nShawn.\n"},{"id":"81340","messageId":"65688C06-BB6A-4E95-A4B9-A1A7C206BE2E@sent.com","threadId":"14167","inReplyTo":"7vskuzq5ix.fsf@gitster.siamese.dyndns.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"","fromEmail":"logank@sent.com","sentAt":"2008-06-26T23:36:27Z","receivedAt":"2008-06-26T23:36:27Z","isPatch":false,"sender":{"key":"logank@sent.com","avatar":"https://gravatar.com/avatar/b7eb73a3eeb83e51f7b9a39a93c798da45e21d2f378efd58fc9139ec128a39f4?d=mp&s=160"},"body":"On Jun 26, 2008, at 1:56 PM, Junio C Hamano wrote:\n\n>> \"The file shouldn't be short unless someone truncated it, or there\n>> is a bug in index-pack. Neither is very likely, but I don't think\n>> we would want to retry pread'ing the same block forever.\n>\n> I don't think we would want to retry even once. Return value of 0 \n> from\n> pread is defined to be an EOF, isn't it?\n\nNo, it seems to be a simple error-out in this case. We have 2.4.20 \nsystems with nfs-utils 0.3.3 and used to frequently get the same error \nwhile pushing. I made a similar change back in February and haven't \nhad a problem since:\n\ndiff --git a/index-pack.c b/index-pack.c\nindex 5ac91ba..85c8bdb 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -313,7 +313,14 @@ static void *get_data_from_pack(struct \nobject_entry *obj)\n\tsrc = xmalloc(len);\n\tdata = src;\n\tdo {\n+\t\t// It appears that if multiple threads read across NFS, the\n+\t\t// second read will fail. I know this is awful, but we wait for\n+\t\t// a little bit and try again.\n\t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n+\t\tif (n <= 0) {\n+\t\t\tsleep(1);\n+\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n+\t\t}\n\t\tif (n <= 0)\n\t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n\t\trdy += n;\n\nI use a sleep request since it seems less likely that the other thread \nwill have an outstanding request after a second of waiting.\n\n-- \n Logan Kennelly\n ,,,\n (. .)\n--ooO-(_)-Ooo--\n"},{"id":"81342","messageId":"7vhcbfojgf.fsf@gitster.siamese.dyndns.org","threadId":"14167","inReplyTo":"65688C06-BB6A-4E95-A4B9-A1A7C206BE2E@sent.com","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-26T23:38:40Z","receivedAt":"2008-06-26T23:38:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"logank@sent.com writes:\n\n> On Jun 26, 2008, at 1:56 PM, Junio C Hamano wrote:\n>\n>>> \"The file shouldn't be short unless someone truncated it, or there\n>>> is a bug in index-pack. Neither is very likely, but I don't think\n>>> we would want to retry pread'ing the same block forever.\n>>\n>> I don't think we would want to retry even once. Return value of 0\n>> from\n>> pread is defined to be an EOF, isn't it?\n>\n> No, it seems to be a simple error-out in this case. We have 2.4.20\n> systems with nfs-utils 0.3.3 and used to frequently get the same error\n> while pushing. I made a similar change back in February and haven't\n> had a problem since:\n>\n> diff --git a/index-pack.c b/index-pack.c\n> index 5ac91ba..85c8bdb 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -313,7 +313,14 @@ static void *get_data_from_pack(struct\n> object_entry *obj)\n> \tsrc = xmalloc(len);\n> \tdata = src;\n> \tdo {\n> +\t\t// It appears that if multiple threads read across NFS, the\n> +\t\t// second read will fail. I know this is awful, but we wait for\n> +\t\t// a little bit and try again.\n> \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> +\t\tif (n <= 0) {\n> +\t\t\tsleep(1);\n> +\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> +\t\t}\n> \t\tif (n <= 0)\n> \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> \t\trdy += n;\n>\n> I use a sleep request since it seems less likely that the other thread\n> will have an outstanding request after a second of waiting.\n\nGaah. Don't we have NFS experts in house? Bruce, perhaps?\n"},{"id":"81353","messageId":"20080627025413.GA19568@fieldses.org","threadId":"14167","inReplyTo":"65688C06-BB6A-4E95-A4B9-A1A7C206BE2E@sent.com","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2008-06-27T02:54:13Z","receivedAt":"2008-06-27T02:54:13Z","isPatch":false,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Thu, Jun 26, 2008 at 04:36:27PM -0700, logank@sent.com wrote:\n> On Jun 26, 2008, at 1:56 PM, Junio C Hamano wrote:\n>\n>>> \"The file shouldn't be short unless someone truncated it, or there\n>>> is a bug in index-pack. Neither is very likely, but I don't think\n>>> we would want to retry pread'ing the same block forever.\n>>\n>> I don't think we would want to retry even once. Return value of 0 \n>> from\n>> pread is defined to be an EOF, isn't it?\n>\n> No, it seems to be a simple error-out in this case. We have 2.4.20 \n> systems\n\nThat version's for the client or the server?\n\n--b.\n\n> with nfs-utils 0.3.3 and used to frequently get the same error \n> while pushing. I made a similar change back in February and haven't had a \n> problem since:\n>\n> diff --git a/index-pack.c b/index-pack.c\n> index 5ac91ba..85c8bdb 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -313,7 +313,14 @@ static void *get_data_from_pack(struct object_entry \n> *obj)\n> \tsrc = xmalloc(len);\n> \tdata = src;\n> \tdo {\n> +\t\t// It appears that if multiple threads read across NFS, the\n> +\t\t// second read will fail. I know this is awful, but we wait for\n> +\t\t// a little bit and try again.\n> \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> +\t\tif (n <= 0) {\n> +\t\t\tsleep(1);\n> +\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> +\t\t}\n> \t\tif (n <= 0)\n> \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> \t\trdy += n;\n>\n> I use a sleep request since it seems less likely that the other thread \n> will have an outstanding request after a second of waiting.\n>\n> -- \n> Logan Kennelly\n> ,,,\n> (. .)\n> --ooO-(_)-Ooo--\n>\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"81354","messageId":"20080627025715.GB19568@fieldses.org","threadId":"14167","inReplyTo":"7vhcbfojgf.fsf@gitster.siamese.dyndns.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2008-06-27T02:57:15Z","receivedAt":"2008-06-27T02:57:15Z","isPatch":false,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Thu, Jun 26, 2008 at 04:38:40PM -0700, Junio C Hamano wrote:\n> logank@sent.com writes:\n> \n> > On Jun 26, 2008, at 1:56 PM, Junio C Hamano wrote:\n> >\n> >>> \"The file shouldn't be short unless someone truncated it, or there\n> >>> is a bug in index-pack. Neither is very likely, but I don't think\n> >>> we would want to retry pread'ing the same block forever.\n> >>\n> >> I don't think we would want to retry even once. Return value of 0\n> >> from\n> >> pread is defined to be an EOF, isn't it?\n> >\n> > No, it seems to be a simple error-out in this case. We have 2.4.20\n> > systems with nfs-utils 0.3.3 and used to frequently get the same error\n> > while pushing. I made a similar change back in February and haven't\n> > had a problem since:\n> >\n> > diff --git a/index-pack.c b/index-pack.c\n> > index 5ac91ba..85c8bdb 100644\n> > --- a/index-pack.c\n> > +++ b/index-pack.c\n> > @@ -313,7 +313,14 @@ static void *get_data_from_pack(struct\n> > object_entry *obj)\n> > \tsrc = xmalloc(len);\n> > \tdata = src;\n> > \tdo {\n> > +\t\t// It appears that if multiple threads read across NFS, the\n> > +\t\t// second read will fail. I know this is awful, but we wait for\n> > +\t\t// a little bit and try again.\n> > \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> > +\t\tif (n <= 0) {\n> > +\t\t\tsleep(1);\n> > +\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> > +\t\t}\n> > \t\tif (n <= 0)\n> > \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> > \t\trdy += n;\n> >\n> > I use a sleep request since it seems less likely that the other thread\n> > will have an outstanding request after a second of waiting.\n> \n> Gaah. Don't we have NFS experts in house? Bruce, perhaps?\n\nTrond, you don't have any idea why a 2.6.9-42.0.8.ELsmp client (2.4.28\nserver) might be returning spurious 0's from pread()?\n\nSeems like everything is happening from that one client--the file isn't\nbeing simultaneously accessed from the server or from another client.\n\n--b.\n"},{"id":"81399","messageId":"8F75DDC9-AECF-4158-B2DC-9B038936B560@gmail.com","threadId":"14167","inReplyTo":"20080627025413.GA19568@fieldses.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Christian Holtje","fromEmail":"docwhat@gmail.com","sentAt":"2008-06-27T13:44:39Z","receivedAt":"2008-06-27T13:44:39Z","isPatch":false,"sender":{"key":"docwhat@gmail.com","avatar":"https://gravatar.com/avatar/b87944ebf1ff49feaeefaa1c19a6e98baee691c531ecb860c2bc4f63b464a395?d=mp&s=160"},"body":"On Jun 26, 2008, at 10:54 PM, J. Bruce Fields wrote:\n> On Thu, Jun 26, 2008 at 04:36:27PM -0700, logank@sent.com wrote:\n>> No, it seems to be a simple error-out in this case. We have 2.4.20\n>> systems\n>\n> That version's for the client or the server?\n\nFor the bug I sent the information is:\nServer:\nLinux dev1 2.4.28 #3 SMP Tue Jan 18 14:59:40 EST 2005 i686 i686 i386 \nGNU/Linux\nRed Hat Linux release 9 (Shrike)\n\nClient:\nLinux dev2 2.6.9-42.0.8.ELsmp #1 SMP Tue Jan 30 12:18:01 EST 2007 \nx86_64 x86_64 x86_64 GNU/Linux\nCentOS release 4.4 (Final)\n\nCiao!\n"},{"id":"81402","messageId":"590C8EE3-B909-4EE5-8EFA-F5B9031A8C05@gmail.com","threadId":"14167","inReplyTo":"65688C06-BB6A-4E95-A4B9-A1A7C206BE2E@sent.com","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Christian Holtje","fromEmail":"docwhat@gmail.com","sentAt":"2008-06-27T13:54:10Z","receivedAt":"2008-06-27T13:54:10Z","isPatch":false,"sender":{"key":"docwhat@gmail.com","avatar":"https://gravatar.com/avatar/b87944ebf1ff49feaeefaa1c19a6e98baee691c531ecb860c2bc4f63b464a395?d=mp&s=160"},"body":"On Jun 26, 2008, at 7:36 PM, logank@sent.com wrote:\n> diff --git a/index-pack.c b/index-pack.c\n> index 5ac91ba..85c8bdb 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -313,7 +313,14 @@ static void *get_data_from_pack(struct \n> object_entry *obj)\n> \tsrc = xmalloc(len);\n> \tdata = src;\n> \tdo {\n> +\t\t// It appears that if multiple threads read across NFS, the\n> +\t\t// second read will fail. I know this is awful, but we wait for\n> +\t\t// a little bit and try again.\n> \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> +\t\tif (n <= 0) {\n> +\t\t\tsleep(1);\n> +\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> +\t\t}\n> \t\tif (n <= 0)\n> \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> \t\trdy += n;\n\nThis does work. But the \"unpacking objects\" phase becomes very slow. \nI had 86 objects and I could see every time it did a sleep because the \ncounter would wait a second (or more) before the next object was \nunpacked.\n\nCiao!\n"},{"id":"81407","messageId":"1214578229.7437.14.camel@localhost","threadId":"14167","inReplyTo":"20080627025715.GB19568@fieldses.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Trond Myklebust","fromEmail":"trond.myklebust@netapp.com","sentAt":"2008-06-27T14:50:29Z","receivedAt":"2008-06-27T14:50:29Z","isPatch":false,"sender":{"key":"trond.myklebust@netapp.com","avatar":null},"body":"On Thu, 2008-06-26 at 22:57 -0400, J. Bruce Fields wrote:\n> On Thu, Jun 26, 2008 at 04:38:40PM -0700, Junio C Hamano wrote:\n> > logank@sent.com writes:\n> > \n> > > On Jun 26, 2008, at 1:56 PM, Junio C Hamano wrote:\n> > >\n> > >>> \"The file shouldn't be short unless someone truncated it, or there\n> > >>> is a bug in index-pack. Neither is very likely, but I don't think\n> > >>> we would want to retry pread'ing the same block forever.\n> > >>\n> > >> I don't think we would want to retry even once. Return value of 0\n> > >> from\n> > >> pread is defined to be an EOF, isn't it?\n> > >\n> > > No, it seems to be a simple error-out in this case. We have 2.4.20\n> > > systems with nfs-utils 0.3.3 and used to frequently get the same error\n> > > while pushing. I made a similar change back in February and haven't\n> > > had a problem since:\n> > >\n> > > diff --git a/index-pack.c b/index-pack.c\n> > > index 5ac91ba..85c8bdb 100644\n> > > --- a/index-pack.c\n> > > +++ b/index-pack.c\n> > > @@ -313,7 +313,14 @@ static void *get_data_from_pack(struct\n> > > object_entry *obj)\n> > > \tsrc = xmalloc(len);\n> > > \tdata = src;\n> > > \tdo {\n> > > +\t\t// It appears that if multiple threads read across NFS, the\n> > > +\t\t// second read will fail. I know this is awful, but we wait for\n> > > +\t\t// a little bit and try again.\n> > > \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> > > +\t\tif (n <= 0) {\n> > > +\t\t\tsleep(1);\n> > > +\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> > > +\t\t}\n> > > \t\tif (n <= 0)\n> > > \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> > > \t\trdy += n;\n> > >\n> > > I use a sleep request since it seems less likely that the other thread\n> > > will have an outstanding request after a second of waiting.\n> > \n> > Gaah. Don't we have NFS experts in house? Bruce, perhaps?\n> \n> Trond, you don't have any idea why a 2.6.9-42.0.8.ELsmp client (2.4.28\n> server) might be returning spurious 0's from pread()?\n> \n> Seems like everything is happening from that one client--the file isn't\n> being simultaneously accessed from the server or from another client.\n\nIs the file only being read, or could there be a simultaneous write to\nthe same file? I'm surmising this could be an effect resulting from\nsimultaneous cache invalidations: prior to Linux 2.6.20 or so, we\nweren't rigorously following the VFS/VM rules for page locking, and so\npage cache invalidation in particular could have some curious\nside-effects.\n\nCheers\n Trond\n-- \nTrond Myklebust\nLinux NFS client maintainer\n\nNetApp\nTrond.Myklebust@netapp.com\nwww.netapp.com\n"},{"id":"81689","messageId":"20080630003203.GJ11793@spearce.org","threadId":"14167","inReplyTo":"1214578229.7437.14.camel@localhost","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-30T00:32:03Z","receivedAt":"2008-06-30T00:32:03Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Trond Myklebust wrote:\n> On Thu, 2008-06-26 at 22:57 -0400, J. Bruce Fields wrote:\n> > On Thu, Jun 26, 2008 at 04:38:40PM -0700, Junio C Hamano wrote:\n> > > logank@sent.com writes:\n> > > \n> > > > On Jun 26, 2008, at 1:56 PM, Junio C Hamano wrote:\n> > > >\n> > > >>> \"The file shouldn't be short unless someone truncated it, or there\n> > > >>> is a bug in index-pack. Neither is very likely, but I don't think\n> > > >>> we would want to retry pread'ing the same block forever.\n> > > >>\n> > > >> I don't think we would want to retry even once. Return value of 0\n> > > >> from\n> > > >> pread is defined to be an EOF, isn't it?\n> > > >\n> > > > No, it seems to be a simple error-out in this case. We have 2.4.20\n> > > > systems with nfs-utils 0.3.3 and used to frequently get the same error\n> > > > while pushing. I made a similar change back in February and haven't\n> > > > had a problem since:\n> > > >\n> > > > diff --git a/index-pack.c b/index-pack.c\n> > > > index 5ac91ba..85c8bdb 100644\n> > > > --- a/index-pack.c\n> > > > +++ b/index-pack.c\n> > > > @@ -313,7 +313,14 @@ static void *get_data_from_pack(struct\n> > > > object_entry *obj)\n> > > > \tsrc = xmalloc(len);\n> > > > \tdata = src;\n> > > > \tdo {\n> > > > +\t\t// It appears that if multiple threads read across NFS, the\n> > > > +\t\t// second read will fail. I know this is awful, but we wait for\n> > > > +\t\t// a little bit and try again.\n> > > > \t\tssize_t n = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> > > > +\t\tif (n <= 0) {\n> > > > +\t\t\tsleep(1);\n> > > > +\t\t\tn = pread(pack_fd, data + rdy, len - rdy, from + rdy);\n> > > > +\t\t}\n> > > > \t\tif (n <= 0)\n> > > > \t\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n> > > > \t\trdy += n;\n> > > >\n> > > > I use a sleep request since it seems less likely that the other thread\n> > > > will have an outstanding request after a second of waiting.\n> > > \n> > > Gaah. Don't we have NFS experts in house? Bruce, perhaps?\n> > \n> > Trond, you don't have any idea why a 2.6.9-42.0.8.ELsmp client (2.4.28\n> > server) might be returning spurious 0's from pread()?\n> > \n> > Seems like everything is happening from that one client--the file isn't\n> > being simultaneously accessed from the server or from another client.\n> \n> Is the file only being read, or could there be a simultaneous write to\n> the same file? I'm surmising this could be an effect resulting from\n> simultaneous cache invalidations: prior to Linux 2.6.20 or so, we\n> weren't rigorously following the VFS/VM rules for page locking, and so\n> page cache invalidation in particular could have some curious\n> side-effects.\n\nThe file was created and opened O_CREAT|O_EXCL|O_RDWR, by this\nprocess, written linearly using write(2), without any lseeks.\nWe kept the file descriptor open and starting issuing pread(2)\ncalls for earlier offsets we had alread written. One of those\nkicks back EOF far too early (and results in this bug report).\n\nNote the only accesses we are using is write(2) and pread(2), and\nonce we start reading we don't ever go back to writing. The pread(2)\ncalls are typically issued in ascending offsets, and we read each\nposition only once. This is to try and take advantage of any\nread-ahead the kernel may be able to do. The pread(2) calls are\nrarely (if ever) on a block/page boundary.\n\nNobody else should know about this file. Its written to a temporary\nname and no other well behaved processes would attempt to read\nthe file until it gets closed and renamed to its final destination.\nWe haven't reached that far in the processing when we get this error,\nso there should be only one file descriptor open on the inode, and\nits the same one that wrote the data.\n\n-- \nShawn.\n"},{"id":"81769","messageId":"alpine.LFD.1.10.0806301457590.19095@xanadu.home","threadId":"14167","inReplyTo":"20080630003203.GJ11793@spearce.org","subject":"Re: pread() over NFS (again) [1.5.5.4]","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-06-30T19:09:23Z","receivedAt":"2008-06-30T19:09:23Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 29 Jun 2008, Shawn O. Pearce wrote:\n\n> Trond Myklebust wrote:\n> > Is the file only being read, or could there be a simultaneous write to\n> > the same file? I'm surmising this could be an effect resulting from\n> > simultaneous cache invalidations: prior to Linux 2.6.20 or so, we\n> > weren't rigorously following the VFS/VM rules for page locking, and so\n> > page cache invalidation in particular could have some curious\n> > side-effects.\n> \n> The file was created and opened O_CREAT|O_EXCL|O_RDWR, by this\n> process, written linearly using write(2), without any lseeks.\n> We kept the file descriptor open and starting issuing pread(2)\n> calls for earlier offsets we had alread written. One of those\n> kicks back EOF far too early (and results in this bug report).\n> \n> Note the only accesses we are using is write(2) and pread(2), and\n> once we start reading we don't ever go back to writing.\n\nThat's not exact. With a thin pack, we continue appending data to the \nfile after a bunch of pread() have occurred. And only after those \npread()'s do we know that we actually have a thin pack.\n\n> The pread(2)\n> calls are typically issued in ascending offsets, and we read each\n> position only once. This is to try and take advantage of any\n> read-ahead the kernel may be able to do.\n\nThat's not exact. The pread() calls are done when resolving deltas, \nhence a base object is read and every deltas based on it are recursively \nresolved to find their SHA1 signature. Then another base is picked up \nand the same process repeated. And in practice all those delta chains \nare all interleaced in the pack file due to the fact that objects are \nstored so to optimize access to recent commits. Therefore they're more \nor less random.\n\n\nNicolas\n"}]}