{"thread":{"id":"8330","subject":"[PATCH v3] Prevent megablobs from gunking up git packs","startedAt":"2007-05-26T19:16:59Z","lastAt":"2007-05-27T15:09:01Z","messageCount":6,"participants":["Dana How","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"43360","messageId":"465887AB.1010001@gmail.com","threadId":"8330","inReplyTo":null,"subject":"[PATCH v3] Prevent megablobs from gunking up git packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-26T19:16:59Z","receivedAt":"2007-05-26T19:16:59Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"\nExtremely large blobs distort general-purpose git packfiles.\nThese megablobs can be either stored in separate \"kept\" packfiles,\nor left as loose objects.  Here we add some features to help\neither approach.\n\nThis patch implements the following:\n1. git pack-objects accepts --max-blob-size=N,  with the effect that\n   only loose blobs less than N KB are written to the packfiles(s).\n   If an already packed blob violates this limit (perhaps these are\n   fast-import packs or max-blob-size was reduced),  it _is_ passed\n   through if from a local pack and no loose copy exists.\n2. git repack inspects repack.maxblobsize .  If set,  its\n   value is passed to git pack-objects on the command line.\n   --max-blob-size=N is also accepted by git repack.\n3. No other git pack-objects caller uses this feature or sees any change.\n\nDuring pack *creation* this minimizes including & deltifying megablobs.\n\nDuring pack *use* this feature helps performance by keeping metadata\nin a single smaller packfile,  and possibly reducing the number of index\nfiles that must be read.  Megablobs could be separately packed,  or\nleft as loose objects.\n\nDocumentation has been updated and operation with pack-object's\n--stdout is prevented.  This patch is based on \"next\".\n\nSigned-off-by: Dana L. How <danahow@gmail.com>\n---\n Documentation/config.txt           |    6 ++++++\n Documentation/git-pack-objects.txt |    5 +++++\n Documentation/git-repack.txt       |    9 +++++++++\n builtin-pack-objects.c             |   33 ++++++++++++++++++++++++++++-----\n cache.h                            |    1 +\n git-repack.sh                      |    9 ++++++++-\n sha1_file.c                        |    2 +-\n 7 files changed, 58 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 179cb17..4a14f05 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -599,6 +599,12 @@ remotes.<group>::\n \tThe list of remotes which are fetched by \"git remote update\n \t<group>\".  See gitlink:git-remote[1].\n \n+repack.maxblobsize::\n+\tPrevent gitlink:git-repack[1] from newly packing blobs larger than\n+\tthe specified number in kB,  unless overridden by --max-blob-size=N switch.\n+\tAffected blobs will still be repacked if from a local pack and no loose\n+\tcopy exists.  Defaults to zero which means no maximum size is in effect.\n+\n repack.usedeltabaseoffset::\n \tAllow gitlink:git-repack[1] to create packs that uses\n \tdelta-base offset.  Defaults to false.\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex cfe127a..9b2e33d 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -85,6 +85,11 @@ base-name::\n \ttimes to get to the necessary object.\n \tThe default value for --window is 10 and --depth is 50.\n \n+--max-blob-size=<n>::\n+\tMaximum size of newly packed blobs, expressed in kB.\n+\tThe default is unlimited.  Affected blobs will still be repacked\n+\tif from a local pack and no loose copy exists.\n+\n --max-pack-size=<n>::\n \tMaximum size of each output packfile, expressed in MiB.\n \tIf specified,  multiple packfiles may be created.\ndiff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\nindex 2847c9b..b9d47e1 100644\n--- a/Documentation/git-repack.txt\n+++ b/Documentation/git-repack.txt\n@@ -65,6 +65,11 @@ OPTIONS\n \tto be applied that many times to get to the necessary object.\n \tThe default value for --window is 10 and --depth is 50.\n \n+--max-blob-size=<n>::\n+\tMaximum size of newly packed blobs, expressed in kB.\n+\tThe default is unlimited.  Affected blobs will still be repacked\n+\tif from a local pack and no loose copy exists.\n+\n --max-pack-size=<n>::\n \tMaximum size of each output packfile, expressed in MiB.\n \tIf specified,  multiple packfiles may be created.\n@@ -84,6 +89,10 @@ be able to read (this includes repositories from which packs can\n be copied out over http or rsync, and people who obtained packs\n that way can try to use older git with it).\n \n+The configuration variable `repack.MaxBlobSize` provides the\n+default for the --max-blob-size option if set.  The latter\n+takes precedence.\n+\n \n Author\n ------\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 19b0aa1..59be849 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -17,7 +17,7 @@\n \n static const char pack_usage[] = \"\\\n git-pack-objects [{ -q | --progress | --all-progress }] [--max-pack-size=N] \\n\\\n-\t[--local] [--incremental] [--window=N] [--depth=N] \\n\\\n+\t[--local] [--incremental] [--window=N] [--depth=N] [--max-blob-size=N]\\n\\\n \t[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \\n\\\n \t[--non-empty] [--revs [--unpacked | --all]*] [--reflog] \\n\\\n \t[--stdout | base-name] [<ref-list | <object-list]\";\n@@ -75,6 +75,7 @@ static int num_preferred_base;\n static struct progress progress_state;\n static int pack_compression_level = Z_DEFAULT_COMPRESSION;\n static int pack_compression_seen;\n+static uint32_t max_blob_size;\n \n /*\n  * The object names in objects array are hashed with this hashtable,\n@@ -371,8 +372,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\tpack_size_limit - write_offset : 0;\n \t\t\t\t/* no if no delta */\n \tint usable_delta =\t!entry->delta ? 0 :\n-\t\t\t\t/* yes if unlimited packfile */\n-\t\t\t\t!pack_size_limit ? 1 :\n \t\t\t\t/* no if base written to previous pack */\n \t\t\t\tentry->delta->offset == (off_t)-1 ? 0 :\n \t\t\t\t/* otherwise double-check written to this\n@@ -408,7 +407,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\tbuf = read_sha1_file(entry->sha1, &type, &size);\n \t\tif (!buf)\n \t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->sha1));\n-\t\tif (size != entry->size)\n+\t\tif (size != entry->size && type == obj_type)\n \t\t\tdie(\"object %s size inconsistency (%lu vs %lu)\",\n \t\t\t    sha1_to_hex(entry->sha1), size, entry->size);\n \t\tif (usable_delta) {\n@@ -564,6 +563,17 @@ static off_t write_one(struct sha1file *f,\n \t\t\treturn 0;\n \t}\n \n+\t/* refuse to include as many megablobs as possible */\n+\tif (max_blob_size && e->size >= max_blob_size) {\n+\t\tstruct stat st;\n+\t\t/* skip if unpacked, remotely packed, or loose anywhere */\n+\t\tif (!e->in_pack || !e->in_pack->pack_local || find_sha1_file(e->sha1, &st)) {\n+\t\t\te->offset = (off_t)-1;\t/* might drop reused delta base if mbs less */\n+\t\t\twritten++;\n+\t\t\treturn offset;\n+\t\t}\n+\t}\n+\n \te->offset = offset;\n \tsize = write_object(f, e, offset);\n \tif (!size) {\n@@ -1422,13 +1432,16 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \n \t/* Now some size filtering heuristics. */\n \ttrg_size = trg_entry->size;\n+\tsrc_size = src_entry->size;\n+\t/* prevent use if could be later dropped from packfile */\n+\tif (max_blob_size && (trg_size >= max_blob_size || src_size >= max_blob_size))\n+\t\treturn 0;\n \tmax_size = trg_size/2 - 20;\n \tmax_size = max_size * (max_depth - src_entry->depth) / max_depth;\n \tif (max_size == 0)\n \t\treturn 0;\n \tif (trg_entry->delta && trg_entry->delta_size <= max_size)\n \t\tmax_size = trg_entry->delta_size-1;\n-\tsrc_size = src_entry->size;\n \tsizediff = src_size < trg_size ? trg_size - src_size : 0;\n \tif (sizediff >= max_size)\n \t\treturn 0;\n@@ -1735,6 +1748,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tincremental = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!prefixcmp(arg, \"--max-blob-size=\")) {\n+\t\t\tchar *end;\n+\t\t\tmax_blob_size = strtoul(arg+16, &end, 0) * 1024;\n+\t\t\tif (!arg[16] || *end)\n+\t\t\t\tusage(pack_usage);\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"--compression=\")) {\n \t\t\tchar *end;\n \t\t\tint level = strtoul(arg+14, &end, 0);\n@@ -1855,6 +1875,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!pack_to_stdout && thin)\n \t\tdie(\"--thin cannot be used to build an indexable pack.\");\n \n+\tif (pack_to_stdout && max_blob_size)\n+\t\tdie(\"--max-blob-size cannot be used to build a pack for transfer.\");\n+\n \tprepare_packed_git();\n \n \tif (progress)\ndiff --git a/cache.h b/cache.h\nindex 4994d03..424b321 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -356,6 +356,7 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename);\n \n extern int has_sha1_pack(const unsigned char *sha1, const char **ignore);\n extern int has_sha1_file(const unsigned char *sha1);\n+extern char *find_sha1_file(const unsigned char *sha1, struct stat *st);\n extern void *map_sha1_file(const unsigned char *sha1, unsigned long *);\n \n extern int has_pack_file(const unsigned char *sha1);\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 4ea6e5b..6b4e1af 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -8,7 +8,7 @@ SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n no_update_info= all_into_one= remove_redundant=\n-local= quiet= no_reuse= extra=\n+local= quiet= no_reuse= extra= max_blob_size=\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n@@ -18,6 +18,7 @@ do\n \t-q)\tquiet=-q ;;\n \t-f)\tno_reuse=--no-reuse-object ;;\n \t-l)\tlocal=--local ;;\n+\t--max-blob-size=*) extra=\"$extra $1\" max_blob_size=t ;;\n \t--max-pack-size=*) extra=\"$extra $1\" ;;\n \t--window=*) extra=\"$extra $1\" ;;\n \t--depth=*) extra=\"$extra $1\" ;;\n@@ -35,6 +36,12 @@ true)\n \textra=\"$extra --delta-base-offset\" ;;\n esac\n \n+# handle blob limiting\n+if [ -z \"$max_blob_size\" ]; then\n+\tmbs=\"`git config --int repack.maxblobsize`\"\n+\t[ -n \"$mbs\" ] && extra=\"$extra --max-blob-size=$mbs\"\n+fi\n+\n PACKDIR=\"$GIT_OBJECT_DIRECTORY/pack\"\n PACKTMP=\"$GIT_OBJECT_DIRECTORY/.tmp-$$-pack\"\n rm -f \"$PACKTMP\"-*\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e4c3288..17e9dbf 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -387,7 +387,7 @@ void prepare_alt_odb(void)\n \tread_info_alternates(get_object_directory(), 0);\n }\n \n-static char *find_sha1_file(const unsigned char *sha1, struct stat *st)\n+char *find_sha1_file(const unsigned char *sha1, struct stat *st)\n {\n \tchar *name = sha1_file_name(sha1);\n \tstruct alternate_object_database *alt;\n-- \n1.5.2.764.g7ae34\n"},{"id":"43376","messageId":"7vwsyvgpvf.fsf@assigned-by-dhcp.cox.net","threadId":"8330","inReplyTo":"465887AB.1010001@gmail.com","subject":"Re: [PATCH v3] Prevent megablobs from gunking up git packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-26T22:51:48Z","receivedAt":"2007-05-26T22:51:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dana How <danahow@gmail.com> writes:\n\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 19b0aa1..59be849 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> ...\n> @@ -371,8 +372,6 @@ static unsigned long write_object(struct sha1file *f,\n>  \t\t\t\tpack_size_limit - write_offset : 0;\n>  \t\t\t\t/* no if no delta */\n>  \tint usable_delta =\t!entry->delta ? 0 :\n> -\t\t\t\t/* yes if unlimited packfile */\n> -\t\t\t\t!pack_size_limit ? 1 :\n>  \t\t\t\t/* no if base written to previous pack */\n>  \t\t\t\tentry->delta->offset == (off_t)-1 ? 0 :\n>  \t\t\t\t/* otherwise double-check written to this\n> @@ -408,7 +407,7 @@ static unsigned long write_object(struct sha1file *f,\n>  \t\tbuf = read_sha1_file(entry->sha1, &type, &size);\n>  \t\tif (!buf)\n>  \t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->sha1));\n> -\t\tif (size != entry->size)\n> +\t\tif (size != entry->size && type == obj_type)\n>  \t\t\tdie(\"object %s size inconsistency (%lu vs %lu)\",\n>  \t\t\t    sha1_to_hex(entry->sha1), size, entry->size);\n>  \t\tif (usable_delta) {\n\nI do not quite get how these two hunks relate to the topic of\nthis patch.  Care to enlighten?\n\n> @@ -564,6 +563,17 @@ static off_t write_one(struct sha1file *f,\n>  \t\t\treturn 0;\n>  \t}\n>  \n> +\t/* refuse to include as many megablobs as possible */\n> +\tif (max_blob_size && e->size >= max_blob_size) {\n> +\t\tstruct stat st;\n> +\t\t/* skip if unpacked, remotely packed, or loose anywhere */\n> +\t\tif (!e->in_pack || !e->in_pack->pack_local || find_sha1_file(e->sha1, &st)) {\n> +\t\t\te->offset = (off_t)-1;\t/* might drop reused delta base if mbs less */\n> +\t\t\twritten++;\n> +\t\t\treturn offset;\n> +\t\t}\n> +\t}\n> +\n>  \te->offset = offset;\n>  \tsize = write_object(f, e, offset);\n>  \tif (!size) {\n\nI thought that you are simply ignoring the \"naughty blobs\"---why\nshould it be done this late in the call sequence?  I haven't\nfollowed the existing code nor your patch closely, but I wonder\nwhy the filtering is simply done inside (or by the caller of)\nadd_object_entry().  You would need to do sha1_object_info()\nmuch earlier than the current code does, though.\n"},{"id":"43383","messageId":"56b7f5510705261648g7d3dc2f6lb68b3a6a8dd10012@mail.gmail.com","threadId":"8330","inReplyTo":"7vwsyvgpvf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v3] Prevent megablobs from gunking up git packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-26T23:48:18Z","receivedAt":"2007-05-26T23:48:18Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/26/07, Junio C Hamano <junkio@cox.net> wrote:\n> Dana How <danahow@gmail.com> writes:\n> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> > @@ -371,8 +372,6 @@ static unsigned long write_object(struct sha1file *f,\n> >                               /* no if no delta */\n> >       int usable_delta =      !entry->delta ? 0 :\n> > -                             /* yes if unlimited packfile */\n> > -                             !pack_size_limit ? 1 :\n> >                               /* no if base written to previous pack */\n> >                               entry->delta->offset == (off_t)-1 ? 0 :\n> >                               /* otherwise double-check written to this\n> > @@ -408,7 +407,7 @@ static unsigned long write_object(struct sha1file *f,\n> >               buf = read_sha1_file(entry->sha1, &type, &size);\n> >               if (!buf)\n> >                       die(\"unable to read %s\", sha1_to_hex(entry->sha1));\n> > -             if (size != entry->size)\n> > +             if (size != entry->size && type == obj_type)\n> >                       die(\"object %s size inconsistency (%lu vs %lu)\",\n> >                           sha1_to_hex(entry->sha1), size, entry->size);\n>\n> I do not quite get how these two hunks relate to the topic of\n> this patch.  Care to enlighten?\n\nNo problem.\n\nWhen the code decides that a blob should not be written to the output file,\nthen I must make sure it is not used as a delta base.  A large blob\nthat triggered the size test and _was_ a delta base could be the result\nof maxblobsize decreasing or being newly specified,\nboth without -f/--no-object-reuse,\nand we need to tolerate the user forgetting the option.\n\nTo make sure that it is not so used,  I re-use the trick from maxpacksize\nwhich ensures that a delta base is not in the previous split pack:\nI set the offset field to -1.  Unfortunately,  I only checked for this magic\nvalue when computing usable_delta if pack_size_limit was set.  It turns\nout the test doesn't need to be conditional on pack_size_limit,  it works\nfor all cases;  so since I need to do the test when maxblobsize was specified\nand maxpacksize wasn't, I deleted the pack_size_limit test.\n\nNow for the second hunk.  The facts above mean we could have marked\nthis entry as a re-used delta, but we are unable to re-use the delta\nbecause its delta base is not being written to this pack.  So we fall into\nthe !to_reuse case even though the size field in the object_entry is the\nsize of the delta,  not the object.  We can detect this by the type coming\nfrom read_sha1_file being unequal to the type set from the pack (which is\none of OBJ_{REF,OFS}_DELTA).  So I disable the size matching\ntest in this case.\n\n> > @@ -564,6 +563,17 @@ static off_t write_one(struct sha1file *f,\n> > +     /* refuse to include as many megablobs as possible */\n> > +     if (max_blob_size && e->size >= max_blob_size) {\n> > +             struct stat st;\n> > +             /* skip if unpacked, remotely packed, or loose anywhere */\n> > +             if (!e->in_pack || !e->in_pack->pack_local || find_sha1_file(e->sha1, &st)) {\n> > +                     e->offset = (off_t)-1;  /* might drop reused delta base if mbs less */\n> > +                     written++;\n> > +                     return offset;\n> > +             }\n> > +     }\n> > +\n>\n> I thought that you are simply ignoring the \"naughty blobs\"---why\n> should it be done this late in the call sequence?  I haven't\n> followed the existing code nor your patch closely, but I wonder\n> why the filtering is simply done inside (or by the caller of)\n> add_object_entry().  You would need to do sha1_object_info()\n> much earlier than the current code does, though.\n\nRecently Nicolas Pitre improved the code as follows:\n(1) tree-walking etc. which calls add_object_entry.\n    We learn sha1, type, name(path), pack&offset, no_try_delta\n    during this step.\n(2) NEW: sort a table of pointers to these objects by pack_offset.\n(3) Now call check_object on each object, but in the order\n     determined in (2).  We learn each object's size during\n     this step.  This requires us to inspect each object's header\n     in the pack(s).\n\nThe result is that we smoothly scan through the pack(s),\ninstead of jumping all over the place.\n\nIf I move sha1_object_info earlier,  before (2),  then I undo\nhis optimization.  This fact ultimately justifies the first two\nhunks that you commented on,  since it means we want\nthe objects to appear in the object list _before_ we can\ndecide not to write them,  and thus we need to handle\nobjects not written and all their consequences\n(which didn't seem too strange to me,\nsince you already have preferred bases).\n\nThanks,\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"},{"id":"43389","messageId":"alpine.LFD.0.99.0705262304200.3366@xanadu.home","threadId":"8330","inReplyTo":"465887AB.1010001@gmail.com","subject":"Re: [PATCH v3] Prevent megablobs from gunking up git packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-27T03:15:16Z","receivedAt":"2007-05-27T03:15:16Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 26 May 2007, Dana How wrote:\n\n> \n> Extremely large blobs distort general-purpose git packfiles.\n> These megablobs can be either stored in separate \"kept\" packfiles,\n> or left as loose objects.  Here we add some features to help\n> either approach.\n> \n> This patch implements the following:\n> 1. git pack-objects accepts --max-blob-size=N,  with the effect that\n>    only loose blobs less than N KB are written to the packfiles(s).\n>    If an already packed blob violates this limit (perhaps these are\n>    fast-import packs or max-blob-size was reduced),  it _is_ passed\n>    through if from a local pack and no loose copy exists.\n\nI'm still not convainced by this feature.  Is it really necessary?\n\nWouldn't it be better if the --max-blob-size=N was instead a \n--trailing-blob-size=N to specify which blobs are considered \"naughty\" \nper our previous discussion? This way there is no incoherency with \nalready packed blobs larger than the treshold that you have to pass \nthrough.\n\nThis, combined with the option to disable deltification of large blobs \n(both options can be specified with the same size), and possibly the \npack size limit, would solve your large blob issue, shouldn't it?\n\n\nNicolas\n"},{"id":"43393","messageId":"56b7f5510705262246o54a38a44xc0c261c4b4161155@mail.gmail.com","threadId":"8330","inReplyTo":"alpine.LFD.0.99.0705262304200.3366@xanadu.home","subject":"Re: [PATCH v3] Prevent megablobs from gunking up git packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-27T05:46:20Z","receivedAt":"2007-05-27T05:46:20Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/26/07, Nicolas Pitre <nico@cam.org> wrote:\n> On Sat, 26 May 2007, Dana How wrote:\n> > Extremely large blobs distort general-purpose git packfiles.\n> > These megablobs can be either stored in separate \"kept\" packfiles,\n> > or left as loose objects.  Here we add some features to help\n> > either approach.\n> >\n> > This patch implements the following:\n> > 1. git pack-objects accepts --max-blob-size=N,  with the effect that\n> >    only loose blobs less than N KB are written to the packfiles(s).\n> >    If an already packed blob violates this limit (perhaps these are\n> >    fast-import packs or max-blob-size was reduced),  it _is_ passed\n> >    through if from a local pack and no loose copy exists.\n>\n> I'm still not convainced by this feature.  Is it really necessary?\n>\n> Wouldn't it be better if the --max-blob-size=N was instead a\n> --trailing-blob-size=N to specify which blobs are considered \"naughty\"\n> per our previous discussion? This way there is no incoherency with\n> already packed blobs larger than the treshold that you have to pass\n> through.\n>\n> This, combined with the option to disable deltification of large blobs\n> (both options can be specified with the same size), and possibly the\n> pack size limit, would solve your large blob issue, shouldn't it?\n\nUnfortunately, it doesn't.\n\nThere are at least three reasonable ways to handle large blobs:\n(1) git-repack -a repacks everything.  Naughty blobs get pushed to\n     the end as discussed (possibly dominating later split packs).\n(2) Naughty blobs accumulate in separate \"kept\" packs.\n     git-repack -a only repacks nice blobs.  Separate scripts,\n     or new options to git-repack,  are needed to repack the \"kept\" packs.\n     A number of people have discussed ideas like this.\n(3) Naughty blobs are kept loose.\n\nWe have 255GB compressed in our Perforce repository and\nit grows by 2GB+ per week.  Although I'm only considering bringing ~10%\nof this into git,  it would be good for me to be able to argue that\nI could bring more.  Every day the equivalent of ~1K+ blobs are committed.\nHow often should I repack the shared repository [that replaces Perforce]?\nWith this level of traffic I believe I should do it every night.\n\nI've been discussing these plans with IT here since they maintain\neverything else.\nThey would like any part of the database that is going to be reorganized\nand replaced to be backed up first.  If only (1) is available,  and I\nrepack every\nnight,  then I need to back up the entire repository every night as well.\nIf I use (2) or (3),  then I back up just the repacked portion each night,\nback up the kept packs only when they are repacked (on a slower schedule),\nand/or back up the loose blobs on a similar schedule.\n\nBesides this back up issue,  I simply don't want to have to repack _all_\nof such a large repository each night.  With (1), nightly repacks get longer\nand longer, and harder to schedule.\n\nI think the minimum features needed to support (2) and (3) are the same:\n(a) An easy way to prevent loose blobs exceeding some size limit\n     from migrating into \"nice\" packs;\n(b) A way to prevent packed objects from being copied when\n     (i) they no longer meet the (new or reduced) size limit AND\n     (ii) they exist in some other safe form in the repository.\nThe behavior of --max-blob-size=N in this patch provides both of these\nwhile deleting other behavior people didn't like.\n\nYou mentioned \"incoherency\" above;\nI'm not too sure how to proceed on that.\nIf you have a more coherent way to provide (a) and (b) above,\nplease let me know.\n\nThanks,\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"},{"id":"43434","messageId":"alpine.LFD.0.99.0705271049280.3366@xanadu.home","threadId":"8330","inReplyTo":"56b7f5510705262246o54a38a44xc0c261c4b4161155@mail.gmail.com","subject":"Re: [PATCH v3] Prevent megablobs from gunking up git packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-27T15:09:01Z","receivedAt":"2007-05-27T15:09:01Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 26 May 2007, Dana How wrote:\n\n> I've been discussing these plans with IT here since they maintain\n> everything else.\n> They would like any part of the database that is going to be reorganized\n> and replaced to be backed up first.  If only (1) is available,  and I\n> repack every\n> night,  then I need to back up the entire repository every night as well.\n\nWhy so?  The initial repack would create a set of packs where the last \npacks to be produced will contain large blobs that you don't have to \never repack.  Or maybe you produce large blobs every day and you want to \nprevent those from entering the pack up front?\n\n> If I use (2) or (3),  then I back up just the repacked portion each night,\n> back up the kept packs only when they are repacked (on a slower schedule),\n> and/or back up the loose blobs on a similar schedule.\n> \n> Besides this back up issue,  I simply don't want to have to repack _all_\n> of such a large repository each night.  With (1), nightly repacks get longer\n> and longer, and harder to schedule.\n> \n> I think the minimum features needed to support (2) and (3) are the same:\n> (a) An easy way to prevent loose blobs exceeding some size limit\n>     from migrating into \"nice\" packs;\n> (b) A way to prevent packed objects from being copied when\n>     (i) they no longer meet the (new or reduced) size limit AND\n>     (ii) they exist in some other safe form in the repository.\n> The behavior of --max-blob-size=N in this patch provides both of these\n> while deleting other behavior people didn't like.\n> \n> You mentioned \"incoherency\" above;\n> I'm not too sure how to proceed on that.\n> If you have a more coherent way to provide (a) and (b) above,\n> please let me know.\n\nI think it boils down to a question of proper wordings.  Describing this \nas max-blob-size is misleading if in the end you still can end up with \nlarger blobs in your pack.  I think there are two solutions to this \nincoherency: either the feature is called something else to reflect the \nfact that it concerns itself only with migration of loose blobs into the \npacked space (I cannot come up with a good name though), or the whole \npack-objects process is aborted with an error whenever the max-blob-size \ncondition cannot be satisfied due to large blobs existing in packed form \nonly indicating that a separate extraction of large blobs process is \nrequired.\n\n\nNicolas\n"}]}