{"thread":{"id":"19360","subject":"Funny error with git gc...","startedAt":"2009-05-15T18:02:26Z","lastAt":"2009-05-24T17:37:58Z","messageCount":18,"participants":["Johannes Schindelin","Junio C Hamano","Avery Pennarun","Linus Torvalds","Shawn O. Pearce","Daniel Barkalow","Matthias Andree"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"114006","messageId":"alpine.DEB.1.00.0905152000520.4449@intel-tinevez-2-302","threadId":"19360","inReplyTo":null,"subject":"Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-15T18:02:26Z","receivedAt":"2009-05-15T18:02:26Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nif you need a chuckle, like me, you might appreciate this story: in one of \nmy repositories, \"git gc\" dies with\n\n\tunable to open object pack directory: ...: Too many open files\n\nturns out that there are a whopping 1088 packs in that repository...\n\nCiao,\nDscho\n"},{"id":"114007","messageId":"7vmy9etey8.fsf@alter.siamese.dyndns.org","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905152000520.4449@intel-tinevez-2-302","subject":"Re: Funny error with git gc...","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-15T18:10:23Z","receivedAt":"2009-05-15T18:10:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> if you need a chuckle, like me, you might appreciate this story: in one of \n> my repositories, \"git gc\" dies with\n>\n> \tunable to open object pack directory: ...: Too many open files\n>\n> turns out that there are a whopping 1088 packs in that repository...\n\nIsn't it a more serious problem than a mere chuckle? How would one\nrecover from such a situation (other than \"mv .git/objects/pack-*;\nfor p in pack-*.pack; do git unpack-objects <$p; done\")?\n"},{"id":"114012","messageId":"32541b130905151124l40194e3eq78411c5b94bd9598@mail.gmail.com","threadId":"19360","inReplyTo":"7vmy9etey8.fsf@alter.siamese.dyndns.org","subject":"Re: Funny error with git gc...","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-05-15T18:24:19Z","receivedAt":"2009-05-15T18:24:19Z","isPatch":false,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Fri, May 15, 2009 at 2:10 PM, Junio C Hamano wrote:\n> Johannes Schindelin writes:\n>> if you need a chuckle, like me, you might appreciate this story: in one of\n>> my repositories, \"git gc\" dies with\n>>\n>>       unable to open object pack directory: ...: Too many open files\n>>\n>> turns out that there are a whopping 1088 packs in that repository...\n>\n> Isn't it a more serious problem than a mere chuckle?  How would one\n> recover from such a situation (other than \"mv .git/objects/pack-*;\n> for p in pack-*.pack; do git unpack-objects <$p; done\")?\n\nI guess if you have root access, you could increase your file ulimit\n(-n) temporarily and then repack. Of course that's not actually\naddressing the root cause of the problem.\n\nAvery\n"},{"id":"114014","messageId":"alpine.LFD.2.01.0905151128230.3343@localhost.localdomain","threadId":"19360","inReplyTo":"7vmy9etey8.fsf@alter.siamese.dyndns.org","subject":"Re: Funny error with git gc...","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-15T18:46:32Z","receivedAt":"2009-05-15T18:46:32Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 15 May 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > if you need a chuckle, like me, you might appreciate this story: in one of \n> > my repositories, \"git gc\" dies with\n> >\n> > \tunable to open object pack directory: ...: Too many open files\n> >\n> > turns out that there are a whopping 1088 packs in that repository...\n> \n> Isn't it a more serious problem than a mere chuckle? How would one\n> recover from such a situation (other than \"mv .git/objects/pack-*;\n> for p in pack-*.pack; do git unpack-objects <$p; done\")?\n\nWell, you can probably just increase the file limits and try again. \nDepending on setup, you may need root to do so, though.\n\nI also think you _should_ be able to avoid this by just limiting the pack \nsize usage. IOW, with some packed_git_limit, something like\n\n\t[core]\n\t\tpackedGitWindowSize = 16k\n\t\tpackedGitLimit = 1M\n\nyou should hopefully be able to repack (slowly) even with a low file \ndescriptor limit, because of the total limit on the size.\n\nThat said, I do agree that ulimit doesn't always work on all systems \n(whether due to hard system limits or due to not having permission to \nraise the limits), and playing games with pack limits is non-obvious. We \nshould really try to avoid getting into such a situation. But I think git \nby default avoids it by the auto-gc, no? So you have to disable that \nexplicitly to get into this bad situation.\n\nOne solution - which I think may be the right one regardless - is to not \nuse \"mmap()\" for small packs or small SHA1 files.\n\nmmap is great for random-access multi-use scenarios (and to avoid some \nmemory pressure by allowing sharing of pages), but for anything that is \njust a couple of pages in size, mmap() just adds big overhead with little \nupside. \n\nSo if we use malloc+read for small things, we'd probably avoid this. Now, \nif you have a few thousand _large_ packs, you'd still be screwed, but the \nmost likely reason for having a thousand packfiles is that you did daily \n\"git pull\"s, and have lots and lots of packs that are pretty small.\n\nDscho? What are your pack-file statistics in this case?\n\n\t\tLinus\n"},{"id":"114015","messageId":"alpine.DEB.1.00.0905152040590.26154@pacific.mpi-cbg.de","threadId":"19360","inReplyTo":"7vmy9etey8.fsf@alter.siamese.dyndns.org","subject":"[MAKESHIFT PATCH] Cope better with a _lot_ of packs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-15T18:52:47Z","receivedAt":"2009-05-15T18:52:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nYou might end up with a situation where you have tons of pack files, e.g.\nwhen using hg2git. In this situation, all kinds of operations may \nend up with a \"too many files open\" error. Let's recover gracefully from \nthat.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Fri, 15 May 2009, Junio C Hamano wrote:\n\n\t> Johannes Schindelin writes:\n\t> \n\t> > if you need a chuckle, like me, you might appreciate this \n\t> > story: in one of my repositories, \"git gc\" dies with\n\t> >\n\t> > \tunable to open object pack directory: ...: Too many open files\n\t> >\n\t> > turns out that there are a whopping 1088 packs in that \n\t> > repository...\n\t> \n\t> Isn't it a more serious problem than a mere chuckle?\n\n\tWell, it was kind of morbid humor, I guess. So much so that I \n\tjust had to send off that mail when I encountered the problem.\n\n\t> How would one recover from such a situation (other than\n\t> \"mv .git/objects/pack-*; for p in pack-*.pack; do git unpack-objects\n\t> <$p; done\")?\n\n\tVery easy: one writes a patch.\n\n\tI am sure that I did not catch all problematic sites, but this \n\tpatch works almost, at least.\n\n\tAnother issue is how to include a test for this... I cannot think\n\tof a non-expensive way.\n\n sha1_file.c | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 28bd908..bd5edd8 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -720,6 +720,8 @@ static int open_packed_git_1(struct packed_git *p)\n \t\treturn error(\"packfile %s index unavailable\", p->pack_name);\n \n \tp->pack_fd = open(p->pack_name, O_RDONLY);\n+\twhile (p->pack_fd < 0 && errno == EMFILE && unuse_one_window(p, -1))\n+\t\tp->pack_fd = open(p->pack_name, O_RDONLY);\n \tif (p->pack_fd < 0 || fstat(p->pack_fd, &st))\n \t\treturn -1;\n \n@@ -937,6 +939,8 @@ static void prepare_packed_git_one(char *objdir, int local)\n \tsprintf(path, \"%s/pack\", objdir);\n \tlen = strlen(path);\n \tdir = opendir(path);\n+\twhile (!dir && errno == EMFILE && unuse_one_window(packed_git, -1))\n+\t\tdir = opendir(path);\n \tif (!dir) {\n \t\tif (errno != ENOENT)\n \t\t\terror(\"unable to open object pack directory: %s: %s\",\n@@ -2339,6 +2343,8 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,\n \n \tfilename = sha1_file_name(sha1);\n \tfd = create_tmpfile(tmpfile, sizeof(tmpfile), filename);\n+\twhile (fd < 0 && errno == EMFILE && unuse_one_window(packed_git, -1))\n+\t\tfd = create_tmpfile(tmpfile, sizeof(tmpfile), filename);\n \tif (fd < 0) {\n \t\tif (errno == EACCES)\n \t\t\treturn error(\"insufficient permission for adding an object to repository database %s\\n\", get_object_directory());\n-- \n1.6.3.1.10.g12b3.dirty\n"},{"id":"114019","messageId":"alpine.DEB.1.00.0905152105340.26154@pacific.mpi-cbg.de","threadId":"19360","inReplyTo":"alpine.LFD.2.01.0905151128230.3343@localhost.localdomain","subject":"Re: Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-15T19:08:27Z","receivedAt":"2009-05-15T19:08:27Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 May 2009, Linus Torvalds wrote:\n\n> On Fri, 15 May 2009, Junio C Hamano wrote:\n> \n> > Johannes Schindelin writes:\n> > \n> > > if you need a chuckle, like me, you might appreciate this story: in \n> > > one of my repositories, \"git gc\" dies with\n> > >\n> > > \tunable to open object pack directory: ...: Too many open files\n> > >\n> > > turns out that there are a whopping 1088 packs in that repository...\n> > \n> > Isn't it a more serious problem than a mere chuckle? How would one \n> > recover from such a situation (other than \"mv .git/objects/pack-*; for \n> > p in pack-*.pack; do git unpack-objects <$p; done\")?\n> \n> Well, you can probably just increase the file limits and try again. \n> Depending on setup, you may need root to do so, though.\n> \n> I also think you _should_ be able to avoid this by just limiting the \n> pack size usage. IOW, with some packed_git_limit, something like\n> \n> \t[core]\n> \t\tpackedGitWindowSize = 16k\n> \t\tpackedGitLimit = 1M\n> \n> you should hopefully be able to repack (slowly) even with a low file \n> descriptor limit, because of the total limit on the size.\n\nI don't think so, because the window size has nothing to do with the \namount of open windows, right?\n\n> That said, I do agree that ulimit doesn't always work on all systems \n> (whether due to hard system limits or due to not having permission to \n> raise the limits), and playing games with pack limits is non-obvious. We \n> should really try to avoid getting into such a situation. But I think git \n> by default avoids it by the auto-gc, no? So you have to disable that \n> explicitly to get into this bad situation.\n\nNo, in this case, nothing was disabled. auto-gc did not kick in, probably \ndue to funny Git usage in hg2git.\n\n> One solution - which I think may be the right one regardless - is to not \n> use \"mmap()\" for small packs or small SHA1 files.\n> \n> mmap is great for random-access multi-use scenarios (and to avoid some \n> memory pressure by allowing sharing of pages), but for anything that is \n> just a couple of pages in size, mmap() just adds big overhead with \n> little upside.\n> \n> So if we use malloc+read for small things, we'd probably avoid this. Now, \n> if you have a few thousand _large_ packs, you'd still be screwed, but the \n> most likely reason for having a thousand packfiles is that you did daily \n> \"git pull\"s, and have lots and lots of packs that are pretty small.\n> \n> Dscho? What are your pack-file statistics in this case?\n\nMostly around 50kB.\n\nBut using malloc()+read() to avoid my use case sounds not \nstraight-forward; it is rather a work-around than a proper solution.\n\nFor performance, I agree that malloc()+read() might be a sensible thing in \na lot of cases.\n\nCiao,\nDscho\n"},{"id":"114020","messageId":"alpine.DEB.1.00.0905152111140.26154@pacific.mpi-cbg.de","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905152105340.26154@pacific.mpi-cbg.de","subject":"Re: Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-15T19:12:10Z","receivedAt":"2009-05-15T19:12:10Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 May 2009, Johannes Schindelin wrote:\n\n\n> > \t[core]\n> > \t\tpackedGitWindowSize = 16k\n> > \t\tpackedGitLimit = 1M\n> > \n> > you should hopefully be able to repack (slowly) even with a low file \n> > descriptor limit, because of the total limit on the size.\n> \n> I don't think so, because the window size has nothing to do with the \n> amount of open windows, right?\n\nObviously I overlooked the \"packedGitLimit\" setting. Still, just a \nwork-around, not a solution that tackles the problem heads-on.\n\nSorry for the noise,\nDscho\n"},{"id":"114021","messageId":"alpine.LFD.2.01.0905151212160.3343@localhost.localdomain","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905152105340.26154@pacific.mpi-cbg.de","subject":"Re: Funny error with git gc...","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-15T19:16:57Z","receivedAt":"2009-05-15T19:16:57Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 15 May 2009, Johannes Schindelin wrote:\n> > \n> > \t[core]\n> > \t\tpackedGitWindowSize = 16k\n> > \t\tpackedGitLimit = 1M\n> > \n> > you should hopefully be able to repack (slowly) even with a low file \n> > descriptor limit, because of the total limit on the size.\n> \n> I don't think so, because the window size has nothing to do with the \n> amount of open windows, right?\n\nNo, but the packedGitLimit does.\n\nSo the argument is that you can't fit all that many open windows in 1M. \n\n> No, in this case, nothing was disabled. auto-gc did not kick in, probably \n> due to funny Git usage in hg2git.\n\nAhh. Scripting.\n\n> > Dscho? What are your pack-file statistics in this case?\n> \n> Mostly around 50kB.\n\nCan you give us an approximation for how many are under 16kB or so?\n\nThe 16kB cutoff is where it's usually just better to malloc+read (because \neven if you don't end up using all the 16kB, just a page-fault or two is \nalready more expensive than just doing the memcpy implied in a 16k read() \nsystem call).\n\n> But using malloc()+read() to avoid my use case sounds not \n> straight-forward; it is rather a work-around than a proper solution.\n\nWell, it's a workaround that is correct for other reasons too. So it's \nlikely worth doing.\n\nThe \"proper solution\" is likely to not avoid repacking. Scripted stuff \nthat does that is buggy.\n\n\t\t\tLinus\n"},{"id":"114022","messageId":"20090515191735.GG30527@spearce.org","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905152040590.26154@pacific.mpi-cbg.de","subject":"Re: [MAKESHIFT PATCH] Cope better with a _lot_ of packs","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-05-15T19:17:35Z","receivedAt":"2009-05-15T19:17:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> \n> You might end up with a situation where you have tons of pack files, e.g.\n> when using hg2git. In this situation, all kinds of operations may \n> end up with a \"too many files open\" error. Let's recover gracefully from \n> that.\n> \n> Signed-off-by: Johannes Schindelin \n\nYea, this looks right to me. JGit had a similar problem and now\nmaintains a core.packedGitOpenFiles parameter, set to 128 by default,\nas one of the rules it uses to limit the size of its pack cache area.\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 28bd908..bd5edd8 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -720,6 +720,8 @@ static int open_packed_git_1(struct packed_git *p)\n> \t\treturn error(\"packfile %s index unavailable\", p->pack_name);\n> \n> \tp->pack_fd = open(p->pack_name, O_RDONLY);\n> +\twhile (p->pack_fd < 0 && errno == EMFILE && unuse_one_window(p, -1))\n> +\t\tp->pack_fd = open(p->pack_name, O_RDONLY);\n> \tif (p->pack_fd < 0 || fstat(p->pack_fd, &st))\n> \t\treturn -1;\n> \n> @@ -937,6 +939,8 @@ static void prepare_packed_git_one(char *objdir, int local)\n> \tsprintf(path, \"%s/pack\", objdir);\n> \tlen = strlen(path);\n> \tdir = opendir(path);\n> +\twhile (!dir && errno == EMFILE && unuse_one_window(packed_git, -1))\n> +\t\tdir = opendir(path);\n> \tif (!dir) {\n> \t\tif (errno != ENOENT)\n> \t\t\terror(\"unable to open object pack directory: %s: %s\",\n> @@ -2339,6 +2343,8 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,\n> \n> \tfilename = sha1_file_name(sha1);\n> \tfd = create_tmpfile(tmpfile, sizeof(tmpfile), filename);\n> +\twhile (fd < 0 && errno == EMFILE && unuse_one_window(packed_git, -1))\n> +\t\tfd = create_tmpfile(tmpfile, sizeof(tmpfile), filename);\n> \tif (fd < 0) {\n> \t\tif (errno == EACCES)\n> \t\t\treturn error(\"insufficient permission for adding an object to repository database %s\\n\", get_object_directory());\n> -- \n\n-- \nShawn.\n"},{"id":"114023","messageId":"alpine.DEB.1.00.0905152115220.26154@pacific.mpi-cbg.de","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905152111140.26154@pacific.mpi-cbg.de","subject":"Re: Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-15T19:18:16Z","receivedAt":"2009-05-15T19:18:16Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 May 2009, Johannes Schindelin wrote:\n\n> On Fri, 15 May 2009, Johannes Schindelin wrote:\n> \n> \n> > > \t[core]\n> > > \t\tpackedGitWindowSize = 16k\n> > > \t\tpackedGitLimit = 1M\n> > > \n> > > you should hopefully be able to repack (slowly) even with a low file \n> > > descriptor limit, because of the total limit on the size.\n> > \n> > I don't think so, because the window size has nothing to do with the \n> > amount of open windows, right?\n> \n> Obviously I overlooked the \"packedGitLimit\" setting. Still, just a \n> work-around, not a solution that tackles the problem heads-on.\n\nBut it worked with the work-around, and not with my heads-on approach. \nThe problem is in xmkstemp(), but when I apply this patch on top of my \nMAKESHIFT PATCH:\n\n-- snip --\ndiff --git a/wrapper.c b/wrapper.c\nindex d8efb13..47806ba 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -192,8 +192,13 @@ int xmkstemp(char *template)\n \tint fd;\n \n \tfd = mkstemp(template);\n+\tif (fd < 0) {\n+\t\trelease_pack_memory((size_t)-1ll, -1);\n+\t\tfd = mkstemp(template);\n+\t}\n \tif (fd < 0)\n-\t\tdie(\"Unable to create temporary file: %s\", strerror(errno));\n+\t\tdie(\"Unable to create temporary file '%s': %s\",\n+\t\t\t\ttemplate, strerror(errno));\n \treturn fd;\n }\n \n-- snap --\n\nI get an even funnier error:\n\nfatal: Unable to create temporary file '.git/objects/pack/tmp_pack_rvhvZb': Invalid argument\nerror: failed to run repack\n\nOh well, I'll let it sit over the weekend...\n\nCiao,\nDscho\n"},{"id":"114024","messageId":"alpine.DEB.1.00.0905152122240.26154@pacific.mpi-cbg.de","threadId":"19360","inReplyTo":"alpine.LFD.2.01.0905151212160.3343@localhost.localdomain","subject":"Re: Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-15T19:24:34Z","receivedAt":"2009-05-15T19:24:34Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 May 2009, Linus Torvalds wrote:\n\n> On Fri, 15 May 2009, Johannes Schindelin wrote:\n>\n> > Linus said:\n> >\n> > > Dscho? What are your pack-file statistics in this case?\n> > \n> > Mostly around 50kB.\n> \n> Can you give us an approximation for how many are under 16kB or so?\n\nNone.\n\nStarts at around 20kB, median is 34kB.\n\n> The \"proper solution\" is likely to not avoid repacking. Scripted stuff \n> that does that is buggy.\n\nYeah, but maybe there are even more clueless people than me running into \nthat situation.\n\nAnyway, off for the weekend,\nDscho\n"},{"id":"114028","messageId":"alpine.LFD.2.01.0905151307390.3343@localhost.localdomain","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905152115220.26154@pacific.mpi-cbg.de","subject":"Re: Funny error with git gc...","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-15T20:08:49Z","receivedAt":"2009-05-15T20:08:49Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 15 May 2009, Johannes Schindelin wrote:\n> \n> \tfd = mkstemp(template);\n> +\tif (fd < 0) {\n> +\t\trelease_pack_memory((size_t)-1ll, -1);\n> +\t\tfd = mkstemp(template);\n\nThis is wrong. You can't use \"template\" twice. You need to re-initialize \nit. The first one will blow away the XXXX pattern.\n\n\t\tLinus\n"},{"id":"114031","messageId":"alpine.LNX.2.00.0905151611541.2147@iabervon.org","threadId":"19360","inReplyTo":"alpine.LFD.2.01.0905151212160.3343@localhost.localdomain","subject":"Re: Funny error with git gc...","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-05-15T20:30:01Z","receivedAt":"2009-05-15T20:30:01Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 15 May 2009, Linus Torvalds wrote:\n\n> On Fri, 15 May 2009, Johannes Schindelin wrote:\n> > > \n> > > \t[core]\n> > > \t\tpackedGitWindowSize = 16k\n> > > \t\tpackedGitLimit = 1M\n> > > \n> > > you should hopefully be able to repack (slowly) even with a low file \n> > > descriptor limit, because of the total limit on the size.\n> > \n> > I don't think so, because the window size has nothing to do with the \n> > amount of open windows, right?\n> \n> No, but the packedGitLimit does.\n> \n> So the argument is that you can't fit all that many open windows in 1M. \n> \n> > No, in this case, nothing was disabled. auto-gc did not kick in, probably \n> > due to funny Git usage in hg2git.\n> \n> Ahh. Scripting.\n> \n> > > Dscho? What are your pack-file statistics in this case?\n> > \n> > Mostly around 50kB.\n> \n> Can you give us an approximation for how many are under 16kB or so?\n> \n> The 16kB cutoff is where it's usually just better to malloc+read (because \n> even if you don't end up using all the 16kB, just a page-fault or two is \n> already more expensive than just doing the memcpy implied in a 16k read() \n> system call).\n> \n> > But using malloc()+read() to avoid my use case sounds not \n> > straight-forward; it is rather a work-around than a proper solution.\n> \n> Well, it's a workaround that is correct for other reasons too. So it's \n> likely worth doing.\n> \n> The \"proper solution\" is likely to not avoid repacking. Scripted stuff \n> that does that is buggy.\n\nI think it's going to be not-too-uncommon in fast-import scripts to want \nto checkpoint periodically so that you don't lose your progress if \ninteracting with the foreign system starts failing partway through, and \nyou'll obviously want to repack afterwards rather than in the middle. But \nif you have different input characteristics than the tool was optimized \nfor, you could end up with a ton of packs by the time it finishes.\n\nObviously, you don't want to avoid repacking before you actually use the \nrepository, but I think it's reasonable to wait until the end of the \nscript to repack, which might actually be too late.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"114163","messageId":"op.ut39s4cf1e62zd@merlin.emma.line.org","threadId":"19360","inReplyTo":"alpine.LFD.2.01.0905151128230.3343@localhost.localdomain","subject":"Re: Funny error with git gc...","fromName":"Matthias Andree","fromEmail":"matthias.andree@gmx.de","sentAt":"2009-05-18T08:28:06Z","receivedAt":"2009-05-18T08:28:06Z","isPatch":false,"sender":{"key":"matthias.andree@gmx.de","avatar":null},"body":"Am 15.05.2009, 20:46 Uhr, schrieb Linus Torvalds \n:\n\n> I also think you _should_ be able to avoid this by just limiting the pack\n> size usage. IOW, with some packed_git_limit, something like\n>\n> \t[core]\n> \t\tpackedGitWindowSize = 16k\n> \t\tpackedGitLimit = 1M\n\nDoes the latter do the equivalent of \"--max-pack-size\"?\n\nIf it does, then there is a related issue on the output side - and that \nis, that I get humongous amounts of packs with that option. Say, I'm \nrepacking my leafnode repos, then\nwith --max-pack-size=1, I get eight packs of 1 MB (8 MB),\n and --max-pack-size=4 yields one pack of 2 - 3 MB size...\n\n-- \nMatthias Andree\n"},{"id":"114380","messageId":"7vzld7tfl2.fsf@alter.siamese.dyndns.org","threadId":"19360","inReplyTo":"20090515191735.GG30527@spearce.org","subject":"Re: [MAKESHIFT PATCH] Cope better with a _lot_ of packs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-21T01:22:33Z","receivedAt":"2009-05-21T01:22:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Johannes Schindelin wrote:\n>> \n>> You might end up with a situation where you have tons of pack files, e.g.\n>> when using hg2git. In this situation, all kinds of operations may \n>> end up with a \"too many files open\" error. Let's recover gracefully from \n>> that.\n>> \n>> Signed-off-by: Johannes Schindelin \n>\n> Yea, this looks right to me. JGit had a similar problem and now\n> maintains a core.packedGitOpenFiles parameter, set to 128 by default,\n> as one of the rules it uses to limit the size of its pack cache area.\n>\n>> diff --git a/sha1_file.c b/sha1_file.c\n>> index 28bd908..bd5edd8 100644\n>> --- a/sha1_file.c\n>> +++ b/sha1_file.c\n>> @@ -720,6 +720,8 @@ static int open_packed_git_1(struct packed_git *p)\n>> \t\treturn error(\"packfile %s index unavailable\", p->pack_name);\n>> \n>> \tp->pack_fd = open(p->pack_name, O_RDONLY);\n>> +\twhile (p->pack_fd < 0 && errno == EMFILE && unuse_one_window(p, -1))\n>> +\t\tp->pack_fd = open(p->pack_name, O_RDONLY);\n>> \tif (p->pack_fd < 0 || fstat(p->pack_fd, &st))\n>> \t\treturn -1;\n\nI wonder if this use of unuse_one_window() can be made more clever to drop\nwindows from the same packfile first. If you have 200 packfiles with 2\nwindows open for each one when you ran out the file descriptors, you do\nnot want to drop one window from each packfile (which would not give you\nany free file descriptor) and then finally release another window (at\nwhich point we have one packfile with no window and we can close the\ndescriptor).\n\nBut at the same time I do not know if it matters in practice. Presumably\nyou have too many packfiles because many of them are very small.\n"},{"id":"114503","messageId":"alpine.DEB.1.00.0905231050480.9099@intel-tinevez-2-302","threadId":"19360","inReplyTo":"alpine.LFD.2.01.0905151307390.3343@localhost.localdomain","subject":"Re: Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-23T08:53:41Z","receivedAt":"2009-05-23T08:53:41Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 May 2009, Linus Torvalds wrote:\n\n> On Fri, 15 May 2009, Johannes Schindelin wrote:\n> > \n> > \tfd = mkstemp(template);\n> > +\tif (fd < 0) {\n> > +\t\trelease_pack_memory((size_t)-1ll, -1);\n> > +\t\tfd = mkstemp(template);\n> \n> This is wrong. You can't use \"template\" twice. You need to re-initialize \n> it. The first one will blow away the XXXX pattern.\n\nTurns out that would be awkward, as the xmkstemp() function would have to \ncopy the template just in case mkstemp() fails due to too many open files, \nand of course it would need to release the copy afterwards.\n\nOTOH we cannot just use the initialized filename, because there \nmight be a race condition with another process, right?\n\nCiao,\nDscho\n"},{"id":"114515","messageId":"alpine.LFD.2.01.0905230859290.3555@localhost.localdomain","threadId":"19360","inReplyTo":"alpine.DEB.1.00.0905231050480.9099@intel-tinevez-2-302","subject":"Re: Funny error with git gc...","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-05-23T16:02:55Z","receivedAt":"2009-05-23T16:02:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 23 May 2009, Johannes Schindelin wrote:\n> On Fri, 15 May 2009, Linus Torvalds wrote:\n> \n> > On Fri, 15 May 2009, Johannes Schindelin wrote:\n> > > \n> > > \tfd = mkstemp(template);\n> > > +\tif (fd < 0) {\n> > > +\t\trelease_pack_memory((size_t)-1ll, -1);\n> > > +\t\tfd = mkstemp(template);\n> > \n> > This is wrong. You can't use \"template\" twice. You need to re-initialize \n> > it. The first one will blow away the XXXX pattern.\n> \n> Turns out that would be awkward, as the xmkstemp() function would have to \n> copy the template just in case mkstemp() fails due to too many open files, \n> and of course it would need to release the copy afterwards.\n> \n> OTOH we cannot just use the initialized filename, because there \n> might be a race condition with another process, right?\n\nCorrect. You basically need to have the caller re-create the template. We \nalready do that in some other cases: see odb_mkstemp(), or \ncreate_tempfile(). Both of them do that template re-creation on failure, \nand try again.\n\nYou could save off the template into a local temporary array, of course.\n\n\t\tLinus\n"},{"id":"114580","messageId":"alpine.DEB.1.00.0905241937350.26154@pacific.mpi-cbg.de","threadId":"19360","inReplyTo":"alpine.LFD.2.01.0905230859290.3555@localhost.localdomain","subject":"Re: Funny error with git gc...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-05-24T17:37:58Z","receivedAt":"2009-05-24T17:37:58Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 23 May 2009, Linus Torvalds wrote:\n\n> On Sat, 23 May 2009, Johannes Schindelin wrote:\n> > On Fri, 15 May 2009, Linus Torvalds wrote:\n> > \n> > > On Fri, 15 May 2009, Johannes Schindelin wrote:\n> > > > \n> > > > \tfd = mkstemp(template);\n> > > > +\tif (fd < 0) {\n> > > > +\t\trelease_pack_memory((size_t)-1ll, -1);\n> > > > +\t\tfd = mkstemp(template);\n> > > \n> > > This is wrong. You can't use \"template\" twice. You need to re-initialize \n> > > it. The first one will blow away the XXXX pattern.\n> > \n> > Turns out that would be awkward, as the xmkstemp() function would have to \n> > copy the template just in case mkstemp() fails due to too many open files, \n> > and of course it would need to release the copy afterwards.\n> > \n> > OTOH we cannot just use the initialized filename, because there \n> > might be a race condition with another process, right?\n> \n> Correct. You basically need to have the caller re-create the template. We \n> already do that in some other cases: see odb_mkstemp(), or \n> create_tempfile(). Both of them do that template re-creation on failure, \n> and try again.\n> \n> You could save off the template into a local temporary array, of course.\n\nI'll let it slip, due to time constraints.\n\nThanks,\nDscho\n"}]}