{"thread":{"id":"8304","subject":"[PATCH] Split packs from git-repack should have descending timestamps","startedAt":"2007-05-24T22:33:50Z","lastAt":"2007-05-25T03:18:25Z","messageCount":5,"participants":["Dana How","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"43190","messageId":"465612CE.4080605@gmail.com","threadId":"8304","inReplyTo":null,"subject":"[PATCH] Split packs from git-repack should have descending timestamps","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-24T22:33:50Z","receivedAt":"2007-05-24T22:33:50Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"\nIf git-repack produces multiple split packs because\n--max-pack-size was in effect, the first pack written\nshould have the latest timestamp because:\n(1) sha1_file.c:rearrange_packed_git() puts more recent\n pack files at the beginning of the search list; and\n(2) the most recent objects are written out first\n while packing.\n\nThis is based on next rather than master to avoid merge\nconflicts with changes already in git-repack.sh due to\nthe --max-pack-size patchset.\n\nSigned-off-by: Dana L. How \n---\n git-repack.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 4ea6e5b..953de4a 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -68,6 +68,7 @@ names=$(git-pack-objects --non-empty --all --reflog $args wrote:\n> \n> If git-repack produces multiple split packs because\n> --max-pack-size was in effect, the first pack written\n> should have the latest timestamp because:\n> (1) sha1_file.c:rearrange_packed_git() puts more recent\n> pack files at the beginning of the search list; and\n> (2) the most recent objects are written out first\n> while packing.\n> \n> This is based on next rather than master to avoid merge\n> conflicts with changes already in git-repack.sh due to\n> the --max-pack-size patchset.\n\nAck. Given our mtime based sorting routine, even without your\nrecent patch to improve it, I think we definately want this type\nof behavior built into git-repack.sh. Good follow-on to your\n--max-pack-size series.\n\n-- \nShawn.\n"},{"id":"43196","messageId":"7vbqg9vhlf.fsf@assigned-by-dhcp.cox.net","threadId":"8304","inReplyTo":"20070525004610.GP28023@spearce.org","subject":"Re: [PATCH] Split packs from git-repack should have descending timestamps","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-25T01:04:44Z","receivedAt":"2007-05-25T01:04:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Dana How wrote:\n>> \n>> If git-repack produces multiple split packs because\n>> --max-pack-size was in effect, the first pack written\n>> should have the latest timestamp because:\n>> (1) sha1_file.c:rearrange_packed_git() puts more recent\n>> pack files at the beginning of the search list; and\n>> (2) the most recent objects are written out first\n>> while packing.\n>> \n>> This is based on next rather than master to avoid merge\n>> conflicts with changes already in git-repack.sh due to\n>> the --max-pack-size patchset.\n>\n> Ack. Given our mtime based sorting routine, even without your\n> recent patch to improve it, I think we definately want this type\n> of behavior built into git-repack.sh. Good follow-on to your\n> --max-pack-size series.\n\nGee, I do not want to touch this, unless we can do something\nabout that sleep 2, even if you have & at the end (actually,\nespecially because you have that -- it makes me worried).\n\nAt the minimum, I think you do not have to restamp at all if the\nresult is a single pack (i.e. the usual case), like so:\n\ncase \"$restamp\" in\n?*' '?*)\n\t# we have more than one.\n # for split packs, the first created should have most recent timestamp\n\tfor file in $restamp ; do touch $file; sleep 2; done &\n\t;;\nesac\n\nCome to think of it, can't you do this \"re-touching\" business at\nthe end of pack-objects without sleeping? You could keep track\nof the names of the packs you produced, and if you have produced\n5, like so:\n\n\t1\n 2\n 3\n 4\n 5\n\nyou would swap timestamp of #1 and #5, #2 and #4 using stat()\nand utime(), and you are done. Each of these huge packs would\ntake more than one second to write it out, but if that is not\nthe case, you could even start with timestamp of #5, subtract 1\nand stamp #4, subtract 1 and stamp #3, ... You may end up using\ntimestamp from the past, but that would not be a problem.\n\nAnd I am really hoping that the other \"use object density in\nreordering\" patch would make this irrelevant. You would have\ncommit and then the rest in the normal input object stream, and\nrecenty ordering done by git-pack-objects should keep commits\ntogether early in the resulting split pack, and earlier parts\nthat have the commits would be hopefully denser.\n"},{"id":"43212","messageId":"56b7f5510705241933x67fd4ed9h6d0e24341c19a9d4@mail.gmail.com","threadId":"8304","inReplyTo":"7vbqg9vhlf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Split packs from git-repack should have descending timestamps","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-25T02:33:40Z","receivedAt":"2007-05-25T02:33:40Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/24/07, Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> > Dana How wrote:\n> >>\n> >> If git-repack produces multiple split packs because\n> >> --max-pack-size was in effect, the first pack written\n> >> should have the latest timestamp because:\n> >> (1) sha1_file.c:rearrange_packed_git() puts more recent\n> >> pack files at the beginning of the search list; and\n> >> (2) the most recent objects are written out first\n> >> while packing.\n> >\n> > Ack. Given our mtime based sorting routine, even without your\n> > recent patch to improve it, I think we definately want this type\n> > of behavior built into git-repack.sh. Good follow-on to your\n> > --max-pack-size series.\n>\n> Gee, I do not want to touch this, unless we can do something\n> about that sleep 2, even if you have & at the end (actually,\n> especially because you have that -- it makes me worried).\n>\n> At the minimum, I think you do not have to restamp at all if the\n> result is a single pack (i.e. the usual case), like so:\n>\n> case \"$restamp\" in\n> ?*' '?*)\n> # we have more than one.\n> # for split packs, the first created should have most recent timestamp\n> for file in $restamp ; do touch $file; sleep 2; done &\n> ;;\n> esac\n>\n> Come to think of it, can't you do this \"re-touching\" business at\n> the end of pack-objects without sleeping? You could keep track\n> of the names of the packs you produced, and if you have produced\n> 5, like so:\n>\n> 1\n> 2\n> 3\n> 4\n> 5\n>\n> you would swap timestamp of #1 and #5, #2 and #4 using stat()\n> and utime(), and you are done. Each of these huge packs would\n> take more than one second to write it out, but if that is not\n> the case, you could even start with timestamp of #5, subtract 1\n> and stamp #4, subtract 1 and stamp #3, ... You may end up using\n> timestamp from the past, but that would not be a problem.\nOK, this triggered the following argument which convinces me:\ngit-pack-objects really should guarantee the correct timestamp\norder, otherwise some other caller will have to repeat the stuff\nI tried to put in git-repack.sh . So I will resubmit following Junio's\nsuggestions. This won't be for a few days.\n\nAlso, if there are rules on allowable bash constructs\n(POSIX only, no &, etc), perhaps they should go in\nSubmittingPatches near the new C99 comments?\n\n> And I am really hoping that the other \"use object density in\n> reordering\" patch would make this irrelevant. You would have\n> commit and then the rest in the normal input object stream, and\n> recenty ordering done by git-pack-objects should keep commits\n> together early in the resulting split pack, and earlier parts\n> that have the commits would be hopefully denser.\nI understand your point, but for a \"normal\" yet extremely\nlarge repository this may not be the case. The \"object density\"\npatch is designed so that the density component of the sort\nkey is extremely weak -- I think the timestamp is very revealing,\nand should be followed in the absence of large variations\nin object density. Correcting the timestamps makes sure\nthat the timestamp order corresponds sensibly to recency order\nwhen packs are split. A sequence of user commands producing\npackfiles results in sensible and usable timestamps;\ni\"d just like to make sure this is also true when packs are\nsplit.\n\nAnyway, I'm not going to submit anything more about\ntimestamps or object density until I see reactions to both patches,\nsince they interact.\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"43216","messageId":"7vhcq1si9q.fsf@assigned-by-dhcp.cox.net","threadId":"8304","inReplyTo":"56b7f5510705241933x67fd4ed9h6d0e24341c19a9d4@mail.gmail.com","subject":"Re: [PATCH] Split packs from git-repack should have descending timestamps","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-25T03:18:25Z","receivedAt":"2007-05-25T03:18:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> Also, if there are rules on allowable bash constructs\n> (POSIX only, no &, etc), perhaps they should go in\n> SubmittingPatches near the new C99 comments?\n\nNo bash arrays, no \"function\" noisewords, limiting in\n${wordword} constructs to POSIX (that means +,-,#,##,%,%%\nbut no regexps), prefer \"test\" over \"[\" (the last one is just for\nreadability).\n\nBut the reason I barfed on \"&\" is not about the syntax nor\nportability. I was afraid of somebody else manipulating things\nlong after the parent \"git-repack\" returns (but still the\nstamper sleeping and waiting to restamp the next one) and gets\nconfused. In this particular case, the restamping is only about\nthe performance so it is not _too_ bad, but in general I really\ndo not like leftover processes still doing something in the\nbackground when the user thinks everything is done.\n\n> I understand your point, but for a \"normal\" yet extremely\n> large repository this may not be the case. The \"object density\"\n> patch is designed so that the density component of the sort\n> key is extremely weak -- I think the timestamp is very revealing,\n> and should be followed in the absence of large variations\n> in object density.\n\nI still think \"a pack that has ONLY megablobs and mark it with\n.keep\" is much simpler approach, and there is no question that\ndensity would work extremely well with that kind of arrangement.\n"}]}