{"thread":{"id":"3978","subject":"bug: git-repack -a -d produces broken pack on NFS","startedAt":"2006-04-27T21:32:07Z","lastAt":"2006-04-28T23:18:53Z","messageCount":9,"participants":["Alex Riesen","Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"19193","messageId":"20060427213207.GA6709@steel.home","threadId":"3978","inReplyTo":null,"subject":"bug: git-repack -a -d produces broken pack on NFS","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-04-27T21:32:07Z","receivedAt":"2006-04-27T21:32:07Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"NFS server: 2.6.15\nClient: 2.6.17-rc2\nmount options: tigra:/home /net/home nfs rw,nosuid,nodev,noatime,vers=3,rsize=8192,wsize=32768,hard,intr,proto=udp,timeo=7,retrans=3,addr=tigra 0 0\n\nRepack protocol ($SRC is /net/home/src):\n\n$SRC/linux.git$ git repack -a -d\nGenerating pack...\nDone counting 235947 objects.\nDeltifying 235947 objects.\n 100% (235947/235947) done\nWriting 235947 objects.\n 100% (235947/235947) done\nTotal 235947, written 235947 (delta 182131), reused 235466 (delta 181650)\nPack pack-6dcda5a7782864d57ec44bd30ebec13b07df2c87 created.\n$SRC/linux.git$ git fsck-objects --full\ngit-fsck-objects: error: Packfile .git/objects/pack/pack-6dcda5a7782864d57ec44bd\n30ebec13b07df2c87.pack SHA1 mismatch with idx\ngit-fsck-objects: fatal: delta data unpack failed\n$SRC/linux.git$ git fsck-objects --full\ngit-fsck-objects: error: Packfile .git/objects/pack/pack-6dcda5a7782864d57ec44bd\n30ebec13b07df2c87.pack SHA1 mismatch with idx\ngit-fsck-objects: fatal: delta data unpack failed\n$SRC/linux.git$ du -sh .\n124M    .\n$SRC/linux.git$ cp . -a $BIG/linux.git\n$SRC/linux.git$ cd $BIG/linux.git\n$BIG/linux.git$ git fsck-objects --full\ngit-fsck-objects: error: Packfile .git/objects/pack/pack-6dcda5a7782864d57ec44bd\n30ebec13b07df2c87.pack SHA1 mismatch with idx\ngit-fsck-objects: fatal: delta data unpack failed\n$BIG/linux.git$ git clone -n . ../tmp\nGenerating pack...\nDone counting 235947 objects.\nDeltifying 235947 objects.\n 100% (235947/235947) done\nTotal 235947, written 235947 (delta 182131), reused 235947 (delta 182131)\n\ngit-index-pack: fatal: packfile '/mnt/large/tmp/raa/tmp/.git/objects/pack/tmp-wc\nRvk5': bad object at offset 102601801: inflate returned -3\ngit-fetch-pack: error: git-fetch-pack: unable to read from git-index-pack\ngit-fetch-pack: error: git-index-pack died with error code 128\nfetch-pack from '/mnt/large/tmp/raa/linux.git/.git' failed.\n\nSo the repository is now _really_ broken.\nI didn't noticed when it started to happen, sorry.\n"},{"id":"19195","messageId":"Pine.LNX.4.64.0604271500500.3701@g5.osdl.org","threadId":"3978","inReplyTo":"20060427213207.GA6709@steel.home","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-04-27T22:11:13Z","receivedAt":"2006-04-27T22:11:13Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 27 Apr 2006, Alex Riesen wrote:\n>\n> NFS server: 2.6.15\n> Client: 2.6.17-rc2\n> mount options: tigra:/home /net/home nfs rw,nosuid,nodev,noatime,vers=3,rsize=8192,wsize=32768,hard,intr,proto=udp,timeo=7,retrans=3,addr=tigra 0 0\n\nIt's repeatable? Can you check if it goes away if your remove \"intr\"?\n\nThat said, the pack-file should all be written with the \"sha1write()\" \ninterface, which is very careful indeed.\n\nI wonder if the _pack-file_ itself might be ok, and the problem is an \nindex file corruption. For some reason we check the index file first, \nwhich is insane. We should check that the pack-file matches its _own_ SHA1 \nfirst, and check the index file second.\n\nSo the appended patch would be sensible: before we bother to look at the \nindex file at all, we should check the pack-file itself.\n\nIf it's just the index file that is corrupt, you may even have a chance to \nrecover the data.\n\nThe index file is also written with sha1write(), though, so I really don't \nsee where it would break. Unless you just simply literally have data \ncorruption on the server for some strange reason.\n\n\t\tLinus\n---\ndiff --git a/pack-check.c b/pack-check.c\nindex 84ed90d..e575879 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -29,12 +29,12 @@ static int verify_packfile(struct packed\n \tpack_base = p->pack_base;\n \tSHA1_Update(&ctx, pack_base, pack_size - 20);\n \tSHA1_Final(sha1, &ctx);\n-\tif (memcmp(sha1, index_base + index_size - 40, 20))\n-\t\treturn error(\"Packfile %s SHA1 mismatch with idx\",\n-\t\t\t     p->pack_name);\n \tif (memcmp(sha1, pack_base + pack_size - 20, 20))\n \t\treturn error(\"Packfile %s SHA1 mismatch with itself\",\n \t\t\t     p->pack_name);\n+\tif (memcmp(sha1, index_base + index_size - 40, 20))\n+\t\treturn error(\"Packfile %s SHA1 mismatch with idx\",\n+\t\t\t     p->pack_name);\n \n \t/* Make sure everything reachable from idx is valid.  Since we\n \t * have verified that nr_objects matches between idx and pack,\n"},{"id":"19196","messageId":"7v4q0ek6i3.fsf@assigned-by-dhcp.cox.net","threadId":"3978","inReplyTo":"Pine.LNX.4.64.0604271500500.3701@g5.osdl.org","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-04-27T22:17:56Z","receivedAt":"2006-04-27T22:17:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> That said, the pack-file should all be written with the \"sha1write()\" \n> interface, which is very careful indeed.\n>\n> I wonder if the _pack-file_ itself might be ok, and the problem is an \n> index file corruption. For some reason we check the index file first, \n> which is insane. We should check that the pack-file matches its _own_ SHA1 \n> first, and check the index file second.\n\nWe need to check both, so I fail to see why the order matters.\n\n> If it's just the index file that is corrupt, you may even have a chance to \n> recover the data.\n>\n> The index file is also written with sha1write(), though, so I really don't \n> see where it would break. Unless you just simply literally have data \n> corruption on the server for some strange reason.\n\nI haven't seen this, and was wondering why.\n\nIndependently, and probably unrelated, but another person\nreported failure while cloning, but the log appeared it had\ntrouble spawning the git-index-pack executable for some reason.\n"},{"id":"19197","messageId":"Pine.LNX.4.64.0604271515490.3701@g5.osdl.org","threadId":"3978","inReplyTo":"Pine.LNX.4.64.0604271500500.3701@g5.osdl.org","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-04-27T22:18:22Z","receivedAt":"2006-04-27T22:18:22Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 27 Apr 2006, Linus Torvalds wrote:\n> \n> I wonder if the _pack-file_ itself might be ok, and the problem is an \n> index file corruption.\n\nHmm. verify_pack() actually checks that the index file matches its own \nSHA1 earlier, so the index file will have passed (my suggested patch is \nstill correct: the same way we check the index file internal integrity \nfirst, we should also check the pack-file internal integrity before we \nbother to cross-check them with each other).\n\nAnyway, the index file SHA1 check means that it's unlikely that the index \nfile was corrupt. But it would be interesting to hear if the pack-file was \ninternally consistent or not.. (Something that git-pack-check didn't check \nin your case, because it checked the pack-file against the index file data \nfirst).\n\n\t\tLinus\n"},{"id":"19198","messageId":"Pine.LNX.4.64.0604271526140.3701@g5.osdl.org","threadId":"3978","inReplyTo":"7v4q0ek6i3.fsf@assigned-by-dhcp.cox.net","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-04-27T22:29:11Z","receivedAt":"2006-04-27T22:29:11Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 27 Apr 2006, Junio C Hamano wrote:\n\n> Linus Torvalds <torvalds@osdl.org> writes:\n> \n> > That said, the pack-file should all be written with the \"sha1write()\" \n> > interface, which is very careful indeed.\n> >\n> > I wonder if the _pack-file_ itself might be ok, and the problem is an \n> > index file corruption. For some reason we check the index file first, \n> > which is insane. We should check that the pack-file matches its _own_ SHA1 \n> > first, and check the index file second.\n> \n> We need to check both, so I fail to see why the order matters.\n\nIt's insane to do any _cross_-file checking before you've even verified \nthat the files themselves are valid.\n\nAnother way of saying the same thing: checking whether the SHA1 of the \npack-file matches the index file is pointless before you've verified that \nthe SHA1 itself is valid.\n\nBasically, if the pack-file is corrupt, you want to know that. You don't \nwant to know that it's SHA1 doesn't match the index file - that's a \n\"secondary\" issue to the fact that the SHA1 wasn't correct in the first \nplace.\n\nRight now, if the pack-file is corrupt, it doesn't actually tell us so. It \nsays that it doesn't match the index file. Which is likely wrong - it \nprobably _does_ match the index file, but it's been corrupted.\n\nSee the difference?\n\n\t\t\tLinus\n"},{"id":"19200","messageId":"7vzmi6iqps.fsf@assigned-by-dhcp.cox.net","threadId":"3978","inReplyTo":"Pine.LNX.4.64.0604271526140.3701@g5.osdl.org","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-04-27T22:44:15Z","receivedAt":"2006-04-27T22:44:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> Right now, if the pack-file is corrupt, it doesn't actually tell us so. It \n> says that it doesn't match the index file. Which is likely wrong - it \n> probably _does_ match the index file, but it's been corrupted.\n>\n> See the difference?\n\nMakes perfect sense.  Thanks.\n"},{"id":"19203","messageId":"Pine.LNX.4.64.0604271630030.3701@g5.osdl.org","threadId":"3978","inReplyTo":"20060427213207.GA6709@steel.home","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-04-27T23:54:34Z","receivedAt":"2006-04-27T23:54:34Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOk, trying to think some more about this..\n\nOn Thu, 27 Apr 2006, Alex Riesen wrote:\n> \n> $SRC/linux.git$ git repack -a -d\n> Generating pack...\n> Done counting 235947 objects.\n> Deltifying 235947 objects.\n>  100% (235947/235947) done\n> Writing 235947 objects.\n>  100% (235947/235947) done\n> Total 235947, written 235947 (delta 182131), reused 235466 (delta 181650)\n> Pack pack-6dcda5a7782864d57ec44bd30ebec13b07df2c87 created.\n> $SRC/linux.git$ git fsck-objects --full\n> git-fsck-objects: error: Packfile .git/objects/pack/pack-6dcda5a7782864d57ec44bd30ebec13b07df2c87.pack SHA1 mismatch with idx\n\nThis is interesting on so many levels.\n\nFirst off, the index file or the pack-file is clearly somehow corrupt, \nbecause when you then try to do the \"git clone\" off the result later on \n(which won't actually check the SHA1's), it gets\n\n> git-index-pack: fatal: packfile '/mnt/large/tmp/raa/tmp/.git/objects/pack/tmp-wcRvk5': bad object at offset 102601801: inflate returned -3\n\nwhich means that either the offset was wrong, or the data at that offset \nwas wrong.\n\nThat made me suspect the object re-use code - it might have been broken in \nthe original pack, and then on re-use the broken data would have been just \ncopied over.\n\nHOWEVER - that doesn't actually fly as an explanation, because even if the \ndata itself was broken, the repack would have re-generated the SHA1, so if \nthe problem had been about copying an already broken pack over, you'd have \ngotten the \"git clone\" error, but you would _not_ have gotten the \"pack \nSHA1 does not match index\" error.\n\nSo in order for the SHA1 to not match, we literally must have corrupted \nthings when we created the pack-file.\n\nHowever, I've stared and stared at the sha1file writing code, and I don't \nsee how you _could_ corrupt it. We use it with interruptible file \ndescriptors all the time (sockets - the exact same code is used to \ntransfer packs over the network), and that \"intr\" shouldn't matter one \nwhit. We're doing very safe things, as far as I can tell.\n\nThe thing is, even if a wild pointer corrupts the write buffer for the \nsha1file writing code somehow, we actually always do the \"calculate the \nSHA1\" and \"flush the buffer to the file\" together. So even if somebody \ncorrupted the buffer, we'd still generate the \"right\" SHA1 (of the \ncorrupted buffer).\n\nSo the only thing that I can see that can generate bad SHA1 checksums is\n - actual problem in the SHA1 buffers themselves (ie a wild pointer \n   corrupting the \"SHA1_CTX\" thing itself)\n - real filesystem corruption. With NFS, the UDP checksums aren't all that \n   strong, but the ethernet CRC should catch things (there have been \n   reports of network cards that don't check the CRC well, but quite \n   frankly, I haven't seen one in a _loong_ time)\n - RAM corruption and/or kernel NFS bugs.\n\nI'll continue to stare at the code, but I can't see anything even remotely \nsuspicious in git itself so far.\n\n\t\tLinus\n"},{"id":"19236","messageId":"20060428222750.GA6462@steel.home","threadId":"3978","inReplyTo":"Pine.LNX.4.64.0604271500500.3701@g5.osdl.org","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-04-28T22:27:50Z","receivedAt":"2006-04-28T22:27:50Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Linus Torvalds, Fri, Apr 28, 2006 00:11:13 +0200:\n> > NFS server: 2.6.15\n> > Client: 2.6.17-rc2\n> > mount options: tigra:/home /net/home nfs rw,nosuid,nodev,noatime,vers=3,rsize=8192,wsize=32768,hard,intr,proto=udp,timeo=7,retrans=3,addr=tigra 0 0\n> \n> It's repeatable? Can you check if it goes away if your remove \"intr\"?\n\nIt does not go away if I remove intr:\n\n    $ grep 'nfs\\>' /proc/mounts\n    tigra:/home /net/home nfs rw,nosuid,nodev,noatime,vers=3,rsize=8192,wsize=32768,hard,proto=udp,timeo=7,retrans=3,addr=tigra 0 0\n\nAnd this is really a broken packfile:\n\n    $ git fsck-objects --full\n    git-fsck-objects: error: Packfile .git/objects/pack/pack-9021635f04e29bb9f3313a54124f64589eca5764.pack SHA1 mismatch with itself\n    git-fsck-objects: fatal: failed to read delta-pack base object a23816d3e9a1684794c8e5a8f1cc0cce26fb61d8\n\nAnd I actually was kind of sure about the hardware (like in: \"it\nworked flawlessly for in the past 2 years\"). Until looked today in the\nlogs and saw this:\n\n    Apr 19 11:49:35 tigra kernel: eth1: tx underrun with maximum tx threshold, txcfg 0xd0f0102e.\n    Apr 19 11:49:35 tigra kernel: eth1: Link wake-up event 0xffffffff\n    Apr 19 11:49:35 tigra kernel: eth1: PCI error 0xf00000\n\nWell, this is actually not _that_ day. And this:\n\n    Apr 28 23:42:19 tigra kernel: eth1: tx underrun with maximum tx threshold, txcfg 0xd0f0102e.\n\nis not exactly the time of most recent test (the one without the \"hard\"\nmount option). But this _is_ that very same interface, and \"PCI error\"\nlooks nasty. Ok, looking at the card... Seats kinda skewed in the\nslot, pressing on it... Wow! (lights go out):\n\n    Apr 29 00:13:35 tigra kernel: eth1: Link wake-up event 0x00020b\n    Apr 29 00:13:35 tigra kernel: eth1: PCI error 0xf00000\n    Apr 29 00:13:39 tigra kernel: NETDEV WATCHDOG: eth1: transmit timed out\n    Apr 29 00:13:39 tigra kernel: eth1: Transmit timed out, status 0x000000, resetting...\n    Apr 29 00:13:39 tigra kernel: eth1: DSPCFG accepted after 0 usec.\n    Apr 29 00:13:39 tigra kernel: eth1: Setting full-duplex based on negotiated link capability.\n\nRedoing test... (Two times only, it's late already):\n\n    $SRC/test2.git$ git repack -a -d\n    Generating pack...\n    Done counting 235775 objects.\n    Deltifying 235775 objects.\n     100% (235775/235775) done\n    Writing 235775 objects.\n     100% (235775/235775) done\n    Total 235775, written 235775 (delta 181885), reused 223766 (delta 171462)\n    Pack pack-9021635f04e29bb9f3313a54124f64589eca5764 created.\n    $SRC/test2.git$ git fsck-objects --full\n    dangling blob 419301f9bff67932cb9551f2d8436b277a3022b0\n    $SRC/test2.git$ git repack -a -d\n    Generating pack...\n    Done counting 235775 objects.\n    Deltifying 235775 objects.\n     100% (235775/235775) done\n    Writing 235775 objects.\n     100% (235775/235775) done\n    Total 235775, written 235775 (delta 181958), reused 235702 (delta 181885)\n    Pack pack-9021635f04e29bb9f3313a54124f64589eca5764 created.\n    $SRC/test2.git$ git fsck-objects --full\n    dangling blob 419301f9bff67932cb9551f2d8436b277a3022b0\n\nHmm... Ok, apologies everyone, I'm just lazy and stupid.\n\nStill, would be nice not to loose a repository just because\nuser is an idiot.\n"},{"id":"19237","messageId":"Pine.LNX.4.64.0604281549380.3701@g5.osdl.org","threadId":"3978","inReplyTo":"20060428222750.GA6462@steel.home","subject":"Re: bug: git-repack -a -d produces broken pack on NFS","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-04-28T23:18:53Z","receivedAt":"2006-04-28T23:18:53Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 29 Apr 2006, Alex Riesen wrote:\n>\n> mount option). But this _is_ that very same interface, and \"PCI error\"\n> looks nasty. Ok, looking at the card... Seats kinda skewed in the\n> slot, pressing on it... Wow! (lights go out):\n> \n>     Apr 29 00:13:35 tigra kernel: eth1: Link wake-up event 0x00020b\n>     Apr 29 00:13:35 tigra kernel: eth1: PCI error 0xf00000\n>     Apr 29 00:13:39 tigra kernel: NETDEV WATCHDOG: eth1: transmit timed out\n>     Apr 29 00:13:39 tigra kernel: eth1: Transmit timed out, status 0x000000, resetting...\n>     Apr 29 00:13:39 tigra kernel: eth1: DSPCFG accepted after 0 usec.\n>     Apr 29 00:13:39 tigra kernel: eth1: Setting full-duplex based on negotiated link capability.\n\nOk, that \"PCI error\" meaning is not entirely clear, but it sounds like \nnoise on the line. The driver just has a comment saying\n\n        /* Hmmmmm, it's not clear how to recover from PCI faults. */\n\nand it's actually pretty possible (and even likely) that whatever \ncorrupted the pack-file happened on the _send_ side, so by the time we get \na PCI error report, there's already a packet out the door that likely has \ncorrupted data.\n\nNow, corrupted data would normally be caught by the UDP checksum, but:\n\n - a lot of modern cards do the IP checksum on the card. Whether the \n   natsemi driver does or not, I have no clue. If it does, it would always \n   make the checksum match the (corrupted) data.\n\n - the IP-level checksums are really quite weak. Realistically, you really \n   really _really_ want to have link-layer checksums working, but since \n   the link-level checksum is _always_ computed by the card (and since a \n   PCI error would have corrupted the packet data that the card saw), \n   link-level checksums will always have the same error that the data got, \n   and pass.\n\n   So even if we did the UDP checksum in software, it has a reasonable \n   chance of not triggering. It's just 16 bits, and not a _good_ 16 bits \n   at that.\n\nAnyway, definitely looks hw-induced.\n\n> Hmm... Ok, apologies everyone, I'm just lazy and stupid.\n\nHaving flaky hw is just unhappy. I'm glad git caught it quickly.\n\n> Still, would be nice not to loose a repository just because\n> user is an idiot.\n\nWell, there's two sides to this:\n\n - packs are very much designed to be very very dense. That means that \n   even the _slightest_ corruption will likely totally destroy them. Even \n   a single-bit flip likely causes massive damage to an archive.\n\n   This is a direct and unavoidable consequence of compression and \n   density.  A less dense format automatically has a lot more redundant \n   data, and the less dense it is, the more likely you can recover from \n   single-bit errors (or even worse ones).\n\n   So this is the downside to compression and packing. All forms of \n   compression (and we do both traditional stream-compression with zlib \n   and aggressive delta compression between objects) inevitably make the \n   effects of corruption much worse, and much harder to recover from.\n\n   That's the bad part.\n\n - The good part is that replication is obviously trivial, and git is \n   inherently very good at making incremental backups.\n\nNow, we could try to have something that tries to recover as much of a \ncorrupted pack-file as possible. Right now, git-unpack-objects just dies \n(as early as possible) if the pack is corrupt. Having some mode where it \ntries to recover from errors and continue would probably be a good \ndebugging and recovery tool.\n\n\t\tLinus\n"}]}