{"thread":{"id":"38213","subject":"git update-ref --stdin : too many open files","startedAt":"2014-12-20T10:24:04Z","lastAt":"2014-12-29T22:56:54Z","messageCount":8,"participants":["Loic Dachary","Junio C Hamano","Stefan Beller","Jonathan Nieder","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"253877","messageId":"54954E44.1080906@dachary.org","threadId":"38213","inReplyTo":null,"subject":"git update-ref --stdin : too many open files","fromName":"Loic Dachary","fromEmail":"loic@dachary.org","sentAt":"2014-12-20T10:24:04Z","receivedAt":"2014-12-20T10:24:04Z","isPatch":false,"sender":{"key":"loic@dachary.org","avatar":null},"body":"Hi,\n\nSteps to reproduce:\n\n$ git --version\ngit version 1.9.1\n$ wc -l /tmp/1\n9090 /tmp/1\n$ head /tmp/1\ndelete refs/pull/1/head\ncreate refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61\ndelete refs/pull/1/merge\ncreate refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e\n...\n$ ulimit -n\n1024\n$ git update-ref --stdin < /tmp/1\nfatal: Unable to create '/home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too many open files\n$ head -20 /tmp/1 | git update-ref --stdin\n$ echo $?\n0\n\nThe workaround is to increase ulimit -n\n\ngit update-ref --stdin should probably close some files.\n\nCheers\n-- \nLoïc Dachary, Artisan Logiciel Libre\n\n"},{"id":"253945","messageId":"xmqqoaqv8jmi.fsf@gitster.dls.corp.google.com","threadId":"38213","inReplyTo":"54954E44.1080906@dachary.org","subject":"Re: git update-ref --stdin : too many open files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-22T21:22:45Z","receivedAt":"2014-12-22T21:22:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Loic Dachary <loic@dachary.org> writes:\n\n> Hi,\n>\n> Steps to reproduce:\n>\n> $ git --version\n> git version 1.9.1\n> $ wc -l /tmp/1\n> 9090 /tmp/1\n> $ head /tmp/1\n> delete refs/pull/1/head\n> create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61\n> delete refs/pull/1/merge\n> create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e\n> ...\n> $ ulimit -n\n> 1024\n> $ git update-ref --stdin < /tmp/1\n> fatal: Unable to create\n> /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too\n> many open files\n> $ head -20 /tmp/1 | git update-ref --stdin\n> $ echo $?\n> 0\n>\n> The workaround is to increase ulimit -n\n>\n> git update-ref --stdin should probably close some files.\n>\n> Cheers\n\nSounds like the recent \"ref update in a transaction\" issue to me.\n\nStefan, want to take a look?  I think we do need to keep the .lock\nfiles without renaming while in transaction, but we do not have to\nkeep them open, so I suspect that a fix may be to split the commit\nfunction into two (one to close but not rename, the other to\nfinalize by renaming) or something.\n\nAlso the version of transaction series we have queued seem to lock\nthese refs very late in the process, but as we discussed privately\na few weeks ago, we would want to move the lock much earlier, when\nthe first update is attempted.\n"},{"id":"253985","messageId":"5498D66B.5090807@gmail.com","threadId":"38213","inReplyTo":"xmqqoaqv8jmi.fsf@gitster.dls.corp.google.com","subject":"Re: git update-ref --stdin : too many open files","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-12-23T02:41:47Z","receivedAt":"2014-12-23T02:41:47Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 22.12.2014 13:22, Junio C Hamano wrote:\n> Loic Dachary <loic@dachary.org> writes:\n> \n>> Hi,\n>>\n>> Steps to reproduce:\n>>\n>> $ git --version\n>> git version 1.9.1\n>> $ wc -l /tmp/1\n>> 9090 /tmp/1\n>> $ head /tmp/1\n>> delete refs/pull/1/head\n>> create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61\n>> delete refs/pull/1/merge\n>> create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e\n>> ...\n>> $ ulimit -n\n>> 1024\n>> $ git update-ref --stdin < /tmp/1\n>> fatal: Unable to create\n>> /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too\n>> many open files\n>> $ head -20 /tmp/1 | git update-ref --stdin\n>> $ echo $?\n>> 0\n>>\n>> The workaround is to increase ulimit -n\n>>\n>> git update-ref --stdin should probably close some files.\n>>\n>> Cheers\n> \n> Sounds like the recent \"ref update in a transaction\" issue to me.\n> \n> Stefan, want to take a look?  I think we do need to keep the .lock\n> files without renaming while in transaction, but we do not have to\n> keep them open, so I suspect that a fix may be to split the commit\n> function into two (one to close but not rename, the other to\n> finalize by renaming) or something.\n\nSounds reasonable. Though by closing the file we're giving up again a\nbit of safety. If we close the file everyone could tamper with the lock\nfile. (Sure they are not supposed to touch it, but they could)\n\n> \n> Also the version of transaction series we have queued seem to lock\n> these refs very late in the process, but as we discussed privately\n> a few weeks ago, we would want to move the lock much earlier, when\n> the first update is attempted.\n\nI'll look into that tomorrow.\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> \n"},{"id":"253988","messageId":"20141223032228.GZ29365@google.com","threadId":"38213","inReplyTo":"5498D66B.5090807@gmail.com","subject":"Re: git update-ref --stdin : too many open files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-12-23T03:22:28Z","receivedAt":"2014-12-23T03:22:28Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Stefan,\n\nStefan Beller wrote:\n> On 22.12.2014 13:22, Junio C Hamano wrote:\n>> Loic Dachary <loic@dachary.org> writes:\n\n>>> fatal: Unable to create /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too many open files\n[...]\n>> Stefan, want to take a look?  I think we do need to keep the .lock\n>> files without renaming while in transaction, but we do not have to\n>> keep them open, so I suspect that a fix may be to split the commit\n>> function into two (one to close but not rename, the other to\n>> finalize by renaming) or something.\n\nMakes sense.\n\n> Sounds reasonable. Though by closing the file we're giving up again a\n> bit of safety. If we close the file everyone could tamper with the lock\n> file. (Sure they are not supposed to touch it, but they could)\n\nAt least on Linux, keeping a file open doesn't offer any protection\nagainst someone else deleting it.  (It also doesn't offer any\nprotection against someone updating the ref directly. ;-)  Opening the\ncorresponding .lock file with O_EXCL is part of the contract for\nupdating refs.)\n\nThanks,\nJonathan\n"},{"id":"254010","messageId":"xmqq38865pg0.fsf@gitster.dls.corp.google.com","threadId":"38213","inReplyTo":"5498D66B.5090807@gmail.com","subject":"Re: git update-ref --stdin : too many open files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-23T15:57:35Z","receivedAt":"2014-12-23T15:57:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <stefanbeller@gmail.com> writes:\n\n> Sounds reasonable. Though by closing the file we're giving up again a\n> bit of safety. If we close the file everyone could tamper with the lock\n> file. (Sure they are not supposed to touch it, but they could)\n\nThere are locking primitives (SysV mandatory locking) that require\nyou to keep the file you have lock on open for you to retain the\nownership of the lock, and that kind of lock does prevent random\nother processes from simultaneously accessing the locked file.\n\nBut that is not what our locks are designed around; our locks rely\nonly on \"open(O_EXCL|O_CREAT) fails if it already exists\".  And\nbetween keeping a lockfile open and closing but not removing a\nlockfile, there is no difference how the lockfile that still exists\nprevents open(O_EXCL|O_CREAT) by other processes from succeeding.\n\nSo we are not giving up any safety at all, which is a good thing ;-).\n"},{"id":"254064","messageId":"549A20B8.3060805@gmail.com","threadId":"38213","inReplyTo":"xmqqoaqv8jmi.fsf@gitster.dls.corp.google.com","subject":"Re: git update-ref --stdin : too many open files","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-12-24T02:11:04Z","receivedAt":"2014-12-24T02:11:04Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 22.12.2014 13:22, Junio C Hamano wrote:\n> Loic Dachary <loic@dachary.org> writes:\n> \n>> Hi,\n>>\n>> Steps to reproduce:\n>>\n>> $ git --version\n>> git version 1.9.1\n>> $ wc -l /tmp/1\n>> 9090 /tmp/1\n>> $ head /tmp/1\n>> delete refs/pull/1/head\n>> create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61\n>> delete refs/pull/1/merge\n>> create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e\n>> ...\n>> $ ulimit -n\n>> 1024\n>> $ git update-ref --stdin < /tmp/1\n>> fatal: Unable to create\n>> /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too\n>> many open files\n>> $ head -20 /tmp/1 | git update-ref --stdin\n>> $ echo $?\n>> 0\n>>\n>> The workaround is to increase ulimit -n\n>>\n>> git update-ref --stdin should probably close some files.\n>>\n>> Cheers\n> \n> Sounds like the recent \"ref update in a transaction\" issue to me.\n> \n> Stefan, want to take a look?  I think we do need to keep the .lock\n> files without renaming while in transaction, but we do not have to\n> keep them open, so I suspect that a fix may be to split the commit\n> function into two (one to close but not rename, the other to\n> finalize by renaming) or something.\n> \n> Also the version of transaction series we have queued seem to lock\n> these refs very late in the process, but as we discussed privately\n> a few weeks ago, we would want to move the lock much earlier, when\n> the first update is attempted.\n\nSo I decided the first thing to do was to put this case into the test\nsuite. so I typed in good faith:\n\ntest_expect_success 'but does it scale?' '\n\tfor i in $(seq 1 1234567) ; do\n\t\tgit branch branch_${i}\n\t\techo \"delete refs/heads/branch_${i}\" >>large_input\n\tdone\n\tgit update-ref --stdin <large_input\n'\n\nAnd there I have problems with my hard disk having more than a million\nfiles in one directory. So once I get rid of that I'll come up with a\nbetter way to test and fix this problem.\n\nThanks,\nStefan\n"},{"id":"254132","messageId":"54A0AE20.4060806@alum.mit.edu","threadId":"38213","inReplyTo":"549A20B8.3060805@gmail.com","subject":"Re: git update-ref --stdin : too many open files","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-12-29T01:28:00Z","receivedAt":"2014-12-29T01:28:00Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 12/24/2014 03:11 AM, Stefan Beller wrote:\n> On 22.12.2014 13:22, Junio C Hamano wrote:\n>> Loic Dachary <loic@dachary.org> writes:\n>>\n>>> Hi,\n>>>\n>>> Steps to reproduce:\n>>>\n>>> $ git --version\n>>> git version 1.9.1\n>>> $ wc -l /tmp/1\n>>> 9090 /tmp/1\n>>> $ head /tmp/1\n>>> delete refs/pull/1/head\n>>> create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61\n>>> delete refs/pull/1/merge\n>>> create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e\n>>> ...\n>>> $ ulimit -n\n>>> 1024\n>>> $ git update-ref --stdin < /tmp/1\n>>> fatal: Unable to create\n>>> /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too\n>>> many open files\n>>> $ head -20 /tmp/1 | git update-ref --stdin\n>>> $ echo $?\n>>> 0\n>>>\n>>> The workaround is to increase ulimit -n\n>>>\n>>> git update-ref --stdin should probably close some files.\n>>>\n>>> Cheers\n>>\n>> Sounds like the recent \"ref update in a transaction\" issue to me.\n>>\n>> Stefan, want to take a look?  I think we do need to keep the .lock\n>> files without renaming while in transaction, but we do not have to\n>> keep them open, so I suspect that a fix may be to split the commit\n>> function into two (one to close but not rename, the other to\n>> finalize by renaming) or something.\n>>\n>> Also the version of transaction series we have queued seem to lock\n>> these refs very late in the process, but as we discussed privately\n>> a few weeks ago, we would want to move the lock much earlier, when\n>> the first update is attempted.\n> \n> So I decided the first thing to do was to put this case into the test\n> suite. so I typed in good faith:\n> \n> test_expect_success 'but does it scale?' '\n> \tfor i in $(seq 1 1234567) ; do\n> \t\tgit branch branch_${i}\n> \t\techo \"delete refs/heads/branch_${i}\" >>large_input\n> \tdone\n> \tgit update-ref --stdin <large_input\n> '\n> \n> And there I have problems with my hard disk having more than a million\n> files in one directory. So once I get rid of that I'll come up with a\n> better way to test and fix this problem.\n\nI suggest something like the following to demonstrate the failure. Note\nthe attempt to set \"ulimit -l\" to a lower value to make the test more\ntractable. (Given that change, sharding the references, which is also\ndemonstrated here, is perhaps superfluous.)\n\ntest_expect_failure 'big transaction does not burst open file limit' '\n(\n\t# Temporarily consider it a failure if we cannot reduce\n\t# the allowed number of open files:\n\ttest \"$(ulimit -n)\" -le 1024 || ulimit -n 1024 || exit 1\n\tfor i in $(seq 33)\n\tdo\n\t\tfor j in $(seq 32)\n\t\tdo\n\t\t\techo \"create refs/heads/$i/$j HEAD\"\n\t\tdone\n\tdone >large_input &&\n\tgit update-ref --stdin <large_input &&\n\tgit rev-parse --verify -q refs/heads/33/32\n)\n'\n\nAfter the bug is fixed, this can be changed to\n\ntest_expect_success 'big transaction does not burst open file limit' '\n(\n\t# Try to reduce the allowed number of open files, but even if\n\t# that is not possible, run the test anyway:\n\ttest \"$(ulimit -n)\" -le 1024 || ulimit -n 1024\n\tfor i in $(seq 33)\n\tdo\n\t\tfor j in $(seq 32)\n\t\tdo\n\t\t\techo \"create refs/heads/$i/$j HEAD\"\n\t\tdone\n\tdone >large_input &&\n\tgit update-ref --stdin <large_input &&\n\tgit rev-parse --verify -q refs/heads/33/32\n)\n'\n\nIt might make sense to test both the \"create\" and the \"delete\" code\npaths, as their locking sequence differs.\n\nI'm doing some work in this area, so I should be able to work on the\nbugfix in the not-too-distant future. My feeling is that the bug is\nunlikely to affect many current users, though it definitely should be\nfixed before sb/atomic-push is merged.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"},{"id":"254169","messageId":"CAGZ79kZ3xfew4Cfdg8-QFj2rD4zQ1M10NNby--Epr=zoCOh7=w@mail.gmail.com","threadId":"38213","inReplyTo":"54A0AE20.4060806@alum.mit.edu","subject":"Re: git update-ref --stdin : too many open files","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-29T22:56:54Z","receivedAt":"2014-12-29T22:56:54Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Dec 28, 2014 at 5:28 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n>\n> I'm doing some work in this area, so I should be able to work on the\n> bugfix in the not-too-distant future. My feeling is that the bug is\n> unlikely to affect many current users, though it definitely should be\n> fixed before sb/atomic-push is merged.\n>\n\nSo are you heading for the bug fix?\nI'd then drop this off my todo list.\n\nThanks,\nStefan\n"}]}