{"thread":{"id":"34861","subject":"A naive proposal for preventing loose object explosions","startedAt":"2013-09-06T03:23:08Z","lastAt":"2013-09-06T18:12:09Z","messageCount":3,"participants":["mfick@codeaurora.org","Junio C Hamano","Martin Fick"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"226906","messageId":"54d0a7f13b53e15ad485e212057b85a3.squirrel@www.codeaurora.org","threadId":"34861","inReplyTo":null,"subject":"A naive proposal for preventing loose object explosions","fromName":"","fromEmail":"mfick@codeaurora.org","sentAt":null,"receivedAt":"2013-09-06T03:23:08Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"I am imagining what I consider to be a naive approach to preventing\nloose unreachable object explosions.   It may seem a bit heavy handed\nat first, but every conversation so far about this issue seems to have\ndied, so I am looking for a simple incremental improvement to what we\nhave today. I theorize that this approach will provide the same\nprotections (good and bad) against races as using git-repack -A -d\nand git-prune --expire <time> regularly will today.\n\n1a)  Add --prune-packed option to git-repack to force a call to git\nprune-packed, without having to specify the -d option to git-repack.\n\n1b) Add a --keep <marker> option to git-repack which will create a\nkeep file with \"marker\" in it for existing pack files which were\nrepacked (not to the new pack).\n\n1c) Now instead of running:\n\n git-repack -A -d\n\nrun:\n\n git-repack --prune-packed --keep 'prune-when-expired'\n\n\nThis should effectively keep a duplicate copy of all old packfiles\naround, but the new pack file will not have unreferenced objects in\nit.  This is similar to having unreachable loose objects left around,\nbut it also keeps around extra copy(ies) of reachable objects wasting\nsome disk space.  While this will normally consume more disk space in\npack files, it will not explode loose objects, which will likely save\na lot of space when such explosions would have occured.   Of course,\nthis should also prevent the severe performance downsides to these\nexplosions.  Object lookups should likely not get any slower than if\nrepack were not run, and the extra new pack might actually help\nfind some objects quicker.   Safety with respect to unreachable object\nrace conditions should be the same as using git repack -A -d since at\nleast one copy of every object should be kept around during this run?\n\n\nThen:\n\n2a) Add support for passing in a list of pack files to git-repack.\nThis list will then be used as the original \"existing\" list instead\nof finding all packfiles without keeps.\n\n2b) Add an --expire-marked <marker> option to git-prune which will\nfind any pack files with a .keep with \"marker\" in it, and evaluate if\nit meets the --expire time.  If so, it will also call:\n\n   git-repack -a -d <expired-pack-files>...\n\nThis should repack any reachable objects from the <expired-pack-files>\ninto a single new pack file.  This may again cause some reachable\nobject duplication (likely with the same performance affects as the\nfirst git-repack phase above), but unreachable objects from <expired-\npack-files> will now have been pruned as they would have been if they\nhad originally been turned into loose objects.\n\n3) Finally on the next repack cycle the current duplicated reachable\nobjects should likely get fully reconsolidated into a single copy.\n\nDoes this sound like it would work?  I may attempt to construct this\nfor internal use (since it is a bit hacky).  It feels like it could be\ndone mostly with some simple shell modding/wrapping (feels less scary than\nmessing with the core C tools).  I wonder if I a missing some obvious flaw\nto this approach?\n\nThanks for any insights,\n\n-Martin\n"},{"id":"226955","messageId":"xmqq61udzx4p.fsf@gitster.dls.corp.google.com","threadId":"34861","inReplyTo":"54d0a7f13b53e15ad485e212057b85a3.squirrel@www.codeaurora.org","subject":"Re: A naive proposal for preventing loose object explosions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T17:19:02Z","receivedAt":"2013-09-06T17:19:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"mfick@codeaurora.org writes:\n\n> Object lookups should likely not get any slower than if\n> repack were not run, and the extra new pack might actually help\n> find some objects quicker.\n\nIn general, having an extra pack, only to keep objects that you know\nare available in other packs, will make _all_ object accesses, not\njust the ones that are contained in that extra pack, slower.\n\nInstead of mmapping all the .idx files for all the available\npackfiles, we could build a table that records, for each packed\nobject, from which packfile at what offset the data is available to\noptimize the access, but obviously building that in-core table will\ntake time, so it may not be a good trade-off to do so at runtime (a\nprecomputed super-.idx that we can mmap at runtime might be a good\nway forward if that turns out to be the case).\n\n> Does this sound like it would work?\n\nSorry, but it is unclear what problem you are trying to solve.\n\nIs it that you do not like that \"repack -A\" ejects unreferenced\nobjects and makes it loose, which you may have many?\n\nThe loosen_unused_packed_objects() function used by \"repack -A\"\ncalls the force_object_loose() function (actually, it is the sole\ncaller of the function).  If you tweak the latter to stream to a\nsingle new \"graveyard\" packfile and mark it as \"kept until expiry\",\nwould it solve the issue the same way but with much smaller impact?\n\nThere already is an infrastructure available to open a single output\npackfile and send multiple objects to it in bulk-checkin.c, and I am\nwondering if you can take advantage of the framework.  The existing\ninterface to it assumes that the object data is coming from a file\ndescriptor (the interface was built to support bulk-checkin of many\nobjects in an empty repository), and it needs refactoring to allow\nstream_to_pack() to take different kind of data sources in the form\nof stateful callback function, though.\n"},{"id":"226969","messageId":"201309061212.10165.mfick@codeaurora.org","threadId":"34861","inReplyTo":"xmqq61udzx4p.fsf@gitster.dls.corp.google.com","subject":"Re: A naive proposal for preventing loose object explosions","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2013-09-06T18:12:09Z","receivedAt":"2013-09-06T18:12:09Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Friday, September 06, 2013 11:19:02 am Junio C Hamano \nwrote:\n> mfick@codeaurora.org writes:\n> > Object lookups should likely not get any slower than if\n> > repack were not run, and the extra new pack might\n> > actually help find some objects quicker.\n> \n> In general, having an extra pack, only to keep objects\n> that you know are available in other packs, will make\n> _all_ object accesses, not just the ones that are\n> contained in that extra pack, slower.\n\nMy assumption was that if the new pack, with all the \nconsolidated reachable objects in it, happens to be searched \nfirst, it would actually speed things up.  And if it is \nsearched last, then the objects weren't in the other packs \nso how could it have made it slower?  It seems this would \nonly slow down the missing object path?\n\nBut it sounds like all the index files are mmaped up front?  \nThen yes, I can see how it would slow things down.  However, \nit is one only extra (hopefully now well optimized) pack.  \nMy base assumption was that even if it does slow things \ndown, it would likely be unmeasurable and a price worth \npaying to avoid an extreme penalty.\n\n\n> Instead of mmapping all the .idx files for all the\n> available packfiles, we could build a table that\n> records, for each packed object, from which packfile at\n> what offset the data is available to optimize the\n> access, but obviously building that in-core table will\n> take time, so it may not be a good trade-off to do so at\n> runtime (a precomputed super-.idx that we can mmap at\n> runtime might be a good way forward if that turns out to\n> be the case).\n> \n> > Does this sound like it would work?\n> \n> Sorry, but it is unclear what problem you are trying to\n> solve.\n\nI think you guessed it below, I am trying to prevent loose \nobject explosions by keeping unreachable objects around in \npacks (instead of loose) until expiry.  With the current way \nthat pack-objects works, this is the best I could come up \nwith (I said naive). :(\n\nToday the git-repack calls git pack-objects like this:\n\ngit pack-objects --keep-true-parents --honor-pack-keep --\nnon-empty --all --reflog $args </dev/null \"$PACKTMP\"\n\nThis has no mechanism to place unreachable objects in a \npack.  If git pack-objects supported an option which \nstreamed them to a separate file (as you suggest below), \nthat would likely be the main piece needed to avoid the \nheavy-handed approach I was suggesting.  \n\nThe problem is how to define the interface for this?  How do \nwe get the filename of the new unreachable packfile?  Today \nthe name of the new packfile is sent to stdout, would we \njust tack on another name?  That seems like it would break \nsome assumptions?  Maybe it would be OK if it only did that \nwhen an --unreachable flag was added?  Then git-repack could \nbe enhanced to understand that flag and the extra filenames \nit outputs?\n\n\n> Is it that you do not like that \"repack -A\" ejects\n> unreferenced objects and makes it loose, which you may\n> have many?\n\nYes, several times a week we have people pushing the kernel \nto wrong projects, this leads to 4M loose objects. :(  \nWithout a solution for this regular problem, we are very \nscared to move our repos off of SSDs.  This leads to hour \nplus long fetches.\n\n\n> The loosen_unused_packed_objects() function used by\n> \"repack -A\" calls the force_object_loose() function\n> (actually, it is the sole caller of the function).  If\n> you tweak the latter to stream to a single new\n> \"graveyard\" packfile and mark it as \"kept until expiry\",\n> would it solve the issue the same way but with much\n> smaller impact?\n\nYes.\n \n> There already is an infrastructure available to open a\n> single output packfile and send multiple objects to it\n> in bulk-checkin.c, and I am wondering if you can take\n> advantage of the framework.  The existing interface to\n> it assumes that the object data is coming from a file\n> descriptor (the interface was built to support\n> bulk-checkin of many objects in an empty repository),\n> and it needs refactoring to allow stream_to_pack() to\n> take different kind of data sources in the form of\n> stateful callback function, though.\n\nThat feels beyond what I could currently dedicate the time \nto do.  Like I said, my solution is heavy handed but it felt \nsimple enough for me to try.  I can spare the extra disk \nspace and I am not convinced the performance hit would be \nbad.  I would, of course, be delighted if someone else were \nto do what you suggest, but I get that it's my itch...\n\n-Martin\n\n\n-- \nThe Qualcomm Innovation Center, Inc. is a member of Code \nAurora Forum, hosted by The Linux Foundation\n \n"}]}