{"thread":{"id":"14110","subject":"[PATCH 2/3] implement some resilience against pack corruptions","startedAt":"2008-06-24T01:23:39Z","lastAt":"2008-06-24T04:49:55Z","messageCount":4,"participants":["Nicolas Pitre","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"80798","messageId":"alpine.LFD.1.10.0806232122180.2979@xanadu.home","threadId":"14110","inReplyTo":null,"subject":"[PATCH 2/3] implement some resilience against pack corruptions","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-06-24T01:23:39Z","receivedAt":"2008-06-24T01:23:39Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"We should be able to fall back to loose objects or alternative packs when\na pack becomes corrupted.  This is especially true when an object exists\nin one pack only as a delta but its base object is corrupted.  Currently\nthere is no way to retrieve the former object even if the later is\navailable in another pack or loose.\n\nThis patch allows for a delta to be resolved (with a performance cost)\nusing a base object from a source other than the pack where that delta\nis located.  Same thing for non-delta objects: rather than failing\noutright, a search is made in other packs or used loose when the\ncurrently active pack has it but corrupted.\n\nOf course git will become extremely noisy with error messages when that\nhappens.  However, if the operation succeeds nevertheless, a simple\n'git repack -a -f -d' will \"fix\" the corrupted repository given that all\ncorrupted objects have a good duplicate somewhere in the object store,\npossibly manually copied from another source.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n\nYes I've been bitten by the corruption described above...\n\n cache.h     |    2 +\n sha1_file.c |   92 ++++++++++++++++++++++++++++++++++++++++++++++++----------\n 2 files changed, 78 insertions(+), 16 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex a0688c6..8ad29e3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -645,6 +645,8 @@ extern struct packed_git {\n \tconst void *index_data;\n \tsize_t index_size;\n \tuint32_t num_objects;\n+\tuint32_t num_bad_objects;\n+\tunsigned char *bad_object_sha1;\n \tint index_version;\n \ttime_t mtime;\n \tint pack_fd;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex fe4ee3e..ffd31c6 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -818,6 +818,8 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)\n \tp->index_data = NULL;\n \tp->index_size = 0;\n \tp->num_objects = 0;\n+\tp->num_bad_objects = 0;\n+\tp->bad_object_sha1 = NULL;\n \tp->pack_size = st.st_size;\n \tp->next = NULL;\n \tp->windows = NULL;\n@@ -982,6 +984,18 @@ void reprepare_packed_git(void)\n \tprepare_packed_git();\n }\n \n+static void mark_bad_packed_object(struct packed_git *p,\n+\t\t\t\t   const unsigned char *sha1)\n+{\n+\tunsigned i;\n+\tfor (i = 0; i < p->num_bad_objects; i++)\n+\t\tif (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))\n+\t\t\treturn;\n+\tp->bad_object_sha1 = xrealloc(p->bad_object_sha1, 20 * (p->num_bad_objects + 1));\n+\thashcpy(p->bad_object_sha1 + 20 * p->num_bad_objects, sha1);\n+\tp->num_bad_objects++;\n+}\n+\n int check_sha1_signature(const unsigned char *sha1, void *map, unsigned long size, const char *type)\n {\n \tunsigned char real_sha1[20];\n@@ -1300,20 +1314,17 @@ static off_t get_delta_base(struct packed_git *p,\n \t\twhile (c & 128) {\n \t\t\tbase_offset += 1;\n \t\t\tif (!base_offset || MSB(base_offset, 7))\n-\t\t\t\tdie(\"offset value overflow for delta base object\");\n+\t\t\t\treturn 0;  /* overflow */\n \t\t\tc = base_info[used++];\n \t\t\tbase_offset = (base_offset << 7) + (c & 127);\n \t\t}\n \t\tbase_offset = delta_obj_offset - base_offset;\n \t\tif (base_offset >= delta_obj_offset)\n-\t\t\tdie(\"delta base offset out of bound\");\n+\t\t\treturn 0;  /* out of bound */\n \t\t*curpos += used;\n \t} else if (type == OBJ_REF_DELTA) {\n \t\t/* The base entry _must_ be in the same pack */\n \t\tbase_offset = find_pack_entry_one(base_info, p);\n-\t\tif (!base_offset)\n-\t\t\tdie(\"failed to find delta-pack base object %s\",\n-\t\t\t\tsha1_to_hex(base_info));\n \t\t*curpos += 20;\n \t} else\n \t\tdie(\"I am totally screwed\");\n@@ -1406,6 +1417,9 @@ const char *packed_object_info_detail(struct packed_git *p,\n \t\t\treturn typename(type);\n \t\tcase OBJ_OFS_DELTA:\n \t\t\tobj_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);\n+\t\t\tif (!obj_offset)\n+\t\t\t\tdie(\"pack %s contains bad delta base reference\",\n+\t\t\t\t    p->pack_name, type);\n \t\t\tif (*delta_chain_length == 0) {\n \t\t\t\trevidx = find_pack_revindex(p, obj_offset);\n \t\t\t\thashcpy(base_sha1, nth_packed_object_sha1(p, revidx->nr));\n@@ -1600,17 +1614,41 @@ static void *unpack_delta_entry(struct packed_git *p,\n \toff_t base_offset;\n \n \tbase_offset = get_delta_base(p, w_curs, &curpos, *type, obj_offset);\n+\tif (!base_offset) {\n+\t\terror(\"failed to validate delta base reference \"\n+\t\t      \"at offset %\"PRIuMAX\" from %s\",\n+\t\t      (uintmax_t)curpos, p->pack_name);\n+\t\treturn NULL;\n+\t}\n \tbase = cache_or_unpack_entry(p, base_offset, &base_size, type, 0);\n-\tif (!base)\n-\t\tdie(\"failed to read delta base object\"\n-\t\t    \" at %\"PRIuMAX\" from %s\",\n-\t\t    (uintmax_t)base_offset, p->pack_name);\n+\tif (!base) {\n+\t\t/*\n+\t\t * We're probably in deep shit, but let's try to fetch\n+\t\t * the required base anyway from another pack or loose.\n+\t\t * This is costly but should happen only in the presence\n+\t\t * of a corrupted pack, and is better than failing outright.\n+\t\t */\n+\t\tstruct revindex_entry *revidx = find_pack_revindex(p, base_offset);\n+\t\tconst unsigned char *base_sha1 =\n+\t\t\t\t\tnth_packed_object_sha1(p, revidx->nr);\n+\t\terror(\"failed to read delta base object %s\"\n+\t\t      \" at offset %\"PRIuMAX\" from %s\",\n+\t\t      sha1_to_hex(base_sha1), (uintmax_t)base_offset,\n+\t\t      p->pack_name);\n+\t\tmark_bad_packed_object(p, base_sha1);\n+\t\tbase = read_sha1_file(base_sha1, type, &base_size);\n+\t\tif (!base)\n+\t\t\treturn NULL;\n+\t}\n \n \tdelta_data = unpack_compressed_entry(p, w_curs, curpos, delta_size);\n-\tif (!delta_data)\n-\t\tdie(\"failed to unpack compressed delta\"\n-\t\t    \" at %\"PRIuMAX\" from %s\",\n-\t\t    (uintmax_t)curpos, p->pack_name);\n+\tif (!delta_data) {\n+\t\terror(\"failed to unpack compressed delta \"\n+\t\t      \"at offset %\"PRIuMAX\" from %s\",\n+\t\t      (uintmax_t)curpos, p->pack_name);\n+\t\tfree(base);\n+\t\treturn NULL;\n+\t}\n \tresult = patch_delta(base, base_size,\n \t\t\t     delta_data, delta_size,\n \t\t\t     sizep);\n@@ -1642,7 +1680,9 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,\n \t\tdata = unpack_compressed_entry(p, &w_curs, curpos, *sizep);\n \t\tbreak;\n \tdefault:\n-\t\tdie(\"unknown object type %i in %s\", *type, p->pack_name);\n+\t\tdata = NULL;\n+\t\terror(\"unknown object type %i at offset %\"PRIuMAX\" in %s\",\n+\t\t      *type, (uintmax_t)obj_offset, p->pack_name);\n \t}\n \tunuse_pack(&w_curs);\n \treturn data;\n@@ -1788,6 +1828,13 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, cons\n \t\t\t\tgoto next;\n \t\t}\n \n+\t\tif (p->num_bad_objects) {\n+\t\t\tunsigned i;\n+\t\t\tfor (i = 0; i < p->num_bad_objects; i++)\n+\t\t\t\tif (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))\n+\t\t\t\t\tgoto next;\n+\t\t}\n+\n \t\toffset = find_pack_entry_one(sha1, p);\n \t\tif (offset) {\n \t\t\t/*\n@@ -1872,11 +1919,24 @@ static void *read_packed_sha1(const unsigned char *sha1,\n \t\t\t      enum object_type *type, unsigned long *size)\n {\n \tstruct pack_entry e;\n+\tvoid *data;\n \n \tif (!find_pack_entry(sha1, &e, NULL))\n \t\treturn NULL;\n-\telse\n-\t\treturn cache_or_unpack_entry(e.p, e.offset, size, type, 1);\n+\tdata = cache_or_unpack_entry(e.p, e.offset, size, type, 1);\n+\tif (!data) {\n+\t\t/*\n+\t\t * We're probably in deep shit, but let's try to fetch\n+\t\t * the required object anyway from another pack or loose.\n+\t\t * This should happen only in the presence of a corrupted\n+\t\t * pack, and is better than failing outright.\n+\t\t */\n+\t\terror(\"failed to read object %s at offset %\"PRIuMAX\" from %s\",\n+\t\t      sha1_to_hex(sha1), (uintmax_t)e.offset, e.p->pack_name);\n+\t\tmark_bad_packed_object(e.p, sha1);\n+\t\tdata = read_sha1_file(sha1, type, size);\n+\t}\n+\treturn data;\n }\n \n /*\n-- \n1.5.6.GIT\n"},{"id":"80801","messageId":"20080624013331.GR11793@spearce.org","threadId":"14110","inReplyTo":"alpine.LFD.1.10.0806232122180.2979@xanadu.home","subject":"Re: [PATCH 2/3] implement some resilience against pack corruptions","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-24T01:33:31Z","receivedAt":"2008-06-24T01:33:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre <nico@cam.org> wrote:\n> We should be able to fall back to loose objects or alternative packs when\n> a pack becomes corrupted.  This is especially true when an object exists\n> in one pack only as a delta but its base object is corrupted.  Currently\n> there is no way to retrieve the former object even if the later is\n> available in another pack or loose.\n\nDang, nice timing Nico.  We were bitten by something like this at\nday-job a couple of weeks back.  Adding this sort of support went\nonto my internal todo-list.  I'm glad you beat me to it.  :)\n \n> Yes I've been bitten by the corruption described above...\n\nBut sorry to hear about this.\n\nAs usual for your patches, the implementation looks quite sane.\nThanks for taking care of it.\n \n-- \nShawn.\n"},{"id":"80821","messageId":"alpine.LFD.1.10.0806232219020.2979@xanadu.home","threadId":"14110","inReplyTo":"20080624013331.GR11793@spearce.org","subject":"Re: [PATCH 2/3] implement some resilience against pack corruptions","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-06-24T02:20:21Z","receivedAt":"2008-06-24T02:20:21Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 23 Jun 2008, Shawn O. Pearce wrote:\n\n> Nicolas Pitre <nico@cam.org> wrote:\n> > We should be able to fall back to loose objects or alternative packs when\n> > a pack becomes corrupted.  This is especially true when an object exists\n> > in one pack only as a delta but its base object is corrupted.  Currently\n> > there is no way to retrieve the former object even if the later is\n> > available in another pack or loose.\n> \n> Dang, nice timing Nico.  We were bitten by something like this at\n> day-job a couple of weeks back.  Adding this sort of support went\n> onto my internal todo-list.  I'm glad you beat me to it.  :)\n>  \n> > Yes I've been bitten by the corruption described above...\n> \n> But sorry to hear about this.\n\nWell, this is fixed now.  So the theory worked.  ;-)\n\n\nNicolas\n"},{"id":"80826","messageId":"7vhcbje8ss.fsf@gitster.siamese.dyndns.org","threadId":"14110","inReplyTo":"alpine.LFD.1.10.0806232122180.2979@xanadu.home","subject":"Re: [PATCH 2/3] implement some resilience against pack corruptions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-24T04:49:55Z","receivedAt":"2008-06-24T04:49:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> We should be able to fall back to loose objects or alternative packs when\n> a pack becomes corrupted.  This is especially true when an object exists\n> in one pack only as a delta but its base object is corrupted.  Currently\n> there is no way to retrieve the former object even if the later is\n> available in another pack or loose.\n\nAll three patches applied, but with the following squashed in.  Thanks\n\n sha1_file.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex a5cef94..9330bc4 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1418,8 +1418,8 @@ const char *packed_object_info_detail(struct packed_git *p,\n \t\tcase OBJ_OFS_DELTA:\n \t\t\tobj_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);\n \t\t\tif (!obj_offset)\n-\t\t\t\tdie(\"pack %s contains bad delta base reference\",\n-\t\t\t\t    p->pack_name, type);\n+\t\t\t\tdie(\"pack %s contains bad delta base reference of type %s\",\n+\t\t\t\t    p->pack_name, typename(type));\n \t\t\tif (*delta_chain_length == 0) {\n \t\t\t\trevidx = find_pack_revindex(p, obj_offset);\n \t\t\t\thashcpy(base_sha1, nth_packed_object_sha1(p, revidx->nr));\n"}]}