{"thread":{"id":"31821","subject":"push race","startedAt":"2012-10-15T09:14:11Z","lastAt":"2012-10-16T19:09:53Z","messageCount":20,"participants":["Angelo Borsotti","Matthieu Moy","Nguyen Thai Ngoc Duy","Ævar Arnfjörð Bjarmason","demerphq","Marc Branchaud","Jeff King","Shawn Pearce","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"201238","messageId":"CAB9Jk9Be4gGaBXixWN7Xju7N6RGKH+FonhaTbZFJ6uYsJDk8dg@mail.gmail.com","threadId":"31821","inReplyTo":null,"subject":"push race","fromName":"Angelo Borsotti","fromEmail":"angelo.borsotti@gmail.com","sentAt":"2012-10-15T09:14:11Z","receivedAt":"2012-10-15T09:14:11Z","isPatch":false,"sender":{"key":"angelo.borsotti@gmail.com","avatar":null},"body":"Hello,\n\nthe push command checks first if the tips of the branches match those\nof the remote\nreferences, and if it does uploads the snapshot.\nThe checking and the uploading are two distinct operations that should\nbe indivisible.\nSuppose that two workstations are pushing at the same time, and that the push of\nthe first has just checked the tips and found that they are ok, and\n--before-- the\npush command uploads the snapshot, the second workstation checks the tips too.\nThe test would be successful, and both workstation would upload their\nfiles, actually\noverwriting each others'.\nI have browsed push.c, transport.c, connect.c, send-pack.c, but have\nnot found any\nsynchronization that protects the checking and the uploading with some critical\nsections.\nHas some sort of mutual exclusion been implemented, or it is up to the user to\nguarantee that two pushes are not done simultaneously?\n\n-Angelo Borsotti\n"},{"id":"201244","messageId":"vpqd30k806o.fsf@grenoble-inp.fr","threadId":"31821","inReplyTo":"CAB9Jk9Be4gGaBXixWN7Xju7N6RGKH+FonhaTbZFJ6uYsJDk8dg@mail.gmail.com","subject":"Re: push race","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-10-15T11:05:51Z","receivedAt":"2012-10-15T11:05:51Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Angelo Borsotti writes:\n\n> the push command checks first if the tips of the branches match those\n> of the remote references, and if it does uploads the snapshot.\n\nThe update does two things: upload objects to the database, and then\nupdate the reference. Adding objects to the database does not change the\nrepository until the objects are reachable from a ref. Updating the ref\nis usually done giving the expected old sha1, and locks the ref, so it\ncan't change in the meantime.\n\nI don't know this part of the code very well, but check refs.c for the C\npart, and \"git update-ref\" for the plumbing interface.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"201247","messageId":"CAB9Jk9ABh45TQTu_KKxNGrVzSaT2_ed87M1jO-074ptP4F=w9A@mail.gmail.com","threadId":"31821","inReplyTo":"CAPc5daUon3eLTDT=3wo_=rTCJWVe=ufCvmSzrjD=0T17Dxkpqw@mail.gmail.com","subject":"Re: push race","fromName":"Angelo Borsotti","fromEmail":"angelo.borsotti@gmail.com","sentAt":"2012-10-15T11:50:57Z","receivedAt":"2012-10-15T11:50:57Z","isPatch":false,"sender":{"key":"angelo.borsotti@gmail.com","avatar":null},"body":"Hi Junio,\n\nis receive-pack invoked only when using the git or ssh protocols, or is it\ninvoked also when accessing directly a remote repository as a mounted\nfilesystem?\nBut let's see if there are problems also with the other protocols: in\ntransport.c,\nthe function git_transport_push calls first get_remote_heads() to get the tips\nof the branches and then calls send_pack(). I did not study deeply the code, but\nI have the impression that there is nothing that prevents an\ninterleaved execution\nof them by two workstations.\nI had a look to receive-pack, and have seen that it creates lock files\nfor the branches\nit updates. However the lock seems to protect only the updating of\nfiles, not the\nchecking of the tips.\n\n-Angelo\n"},{"id":"201249","messageId":"CACsJy8Aw1iM9BTqv4jmoEK+a1gzKUL0rfVGFsnebofd67LxZew@mail.gmail.com","threadId":"31821","inReplyTo":"vpqd30k806o.fsf@grenoble-inp.fr","subject":"Re: push race","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-10-15T11:53:28Z","receivedAt":"2012-10-15T11:53:28Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 15, 2012 at 6:05 PM, Matthieu Moy\n wrote:\n> Angelo Borsotti writes:\n>\n>> the push command checks first if the tips of the branches match those\n>> of the remote references, and if it does uploads the snapshot.\n>\n> The update does two things: upload objects to the database, and then\n> update the reference. Adding objects to the database does not change the\n> repository until the objects are reachable from a ref. Updating the ref\n> is usually done giving the expected old sha1, and locks the ref, so it\n> can't change in the meantime.\n>\n> I don't know this part of the code very well, but check refs.c for the C\n> part, and \"git update-ref\" for the plumbing interface.\n\nI think it's lock_any_ref_for_update(), which is called inside\nrefs.c:update_ref().\n-- \nDuy\n"},{"id":"201256","messageId":"CACBZZX5keWVDZ-rvQfHFChKRC1YwXcUvfiqzgeMjVTydnQCdmg@mail.gmail.com","threadId":"31821","inReplyTo":"CAB9Jk9Be4gGaBXixWN7Xju7N6RGKH+FonhaTbZFJ6uYsJDk8dg@mail.gmail.com","subject":"Re: push race","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-10-15T14:09:40Z","receivedAt":"2012-10-15T14:09:40Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Oct 15, 2012 at 11:14 AM, Angelo Borsotti\n wrote:\n> Hello,\n\nFWIW we have a lot of lemmings pushing to the same ref all the time at\n$work, and while I've seen cases where:\n\n 1. Two clients try to push\n 2. They both get the initial lock\n 3. One of them fails to get the secondary lock (I think updating the ref)\n\nI've never seen cases where they clobber each other in #3 (and I would\nhave known from \"dude, where's my commit that I just pushed\" reports).\n\nSo while we could fix git to make sure there's no race condition such\nthat two clients never get the #2 lock I haven't seen it cause actual\ndata issues because of two clients getting the #3 lock.\n\nIt might still happen in some cases, I recommend testing it with e.g.\nlots of pushes in parallel with GNU Parallel.\n"},{"id":"201257","messageId":"CANgJU+Vq7vnJh1NsZDh0mnTyHQba+oq3=MiwsWL5E5Wb0RiiAg@mail.gmail.com","threadId":"31821","inReplyTo":"CACBZZX5keWVDZ-rvQfHFChKRC1YwXcUvfiqzgeMjVTydnQCdmg@mail.gmail.com","subject":"Re: push race","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2012-10-15T14:13:50Z","receivedAt":"2012-10-15T14:13:50Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 15 October 2012 16:09, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Oct 15, 2012 at 11:14 AM, Angelo Borsotti\n> wrote:\n>> Hello,\n>\n> FWIW we have a lot of lemmings pushing to the same ref all the time at\n> $work, and while I've seen cases where:\n>\n> 1. Two clients try to push\n> 2. They both get the initial lock\n> 3. One of them fails to get the secondary lock (I think updating the ref)\n>\n> I've never seen cases where they clobber each other in #3 (and I would\n> have known from \"dude, where's my commit that I just pushed\" reports).\n\nExcept that the error message is really cryptic. It definitely doesnt\nshout out \"maybe you collided with someone elses push\".\n\nYves\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"201259","messageId":"507C1DB4.2010000@xiplink.com","threadId":"31821","inReplyTo":"CACBZZX5keWVDZ-rvQfHFChKRC1YwXcUvfiqzgeMjVTydnQCdmg@mail.gmail.com","subject":"Re: push race","fromName":"Marc Branchaud","fromEmail":"mbranchaud@xiplink.com","sentAt":"2012-10-15T14:29:08Z","receivedAt":"2012-10-15T14:29:08Z","isPatch":false,"sender":{"key":"mbranchaud@xiplink.com","avatar":null},"body":"On 12-10-15 10:09 AM, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Oct 15, 2012 at 11:14 AM, Angelo Borsotti\n> wrote:\n>> Hello,\n> \n> FWIW we have a lot of lemmings pushing to the same ref all the time at\n> $work, and while I've seen cases where:\n> \n> 1. Two clients try to push\n> 2. They both get the initial lock\n> 3. One of them fails to get the secondary lock (I think updating the ref)\n> \n> I've never seen cases where they clobber each other in #3 (and I would\n> have known from \"dude, where's my commit that I just pushed\" reports).\n> \n> So while we could fix git to make sure there's no race condition such\n> that two clients never get the #2 lock I haven't seen it cause actual\n> data issues because of two clients getting the #3 lock.\n> \n> It might still happen in some cases, I recommend testing it with e.g.\n> lots of pushes in parallel with GNU Parallel.\n\nHere's a previous discussion of a race in concurrent updates to the same ref,\neven when the updates are all identical:\n\nhttp://news.gmane.org/find-root.php?group=gmane.comp.version-control.git&article=164636\n\nIn that thread, Peff outlines the lock procedure for refs:\n\n 1. get the lock\n 2. check and remember the sha1\n 3. release the lock\n 4. do some long-running work (like the actual push)\n 5. get the lock\n 6. check that the sha1 is the same as the remembered one\n 7. update the sha1\n 8. release the lock\n\nAngelo, in your case I think one of your concurrent updates would fail in\nstep 6. As you say, this is after the changes have been uploaded. However,\nthere's none of the file-overwriting that you fear, because the changes are\nstored in git's object database under their SHA hashes. So there'll only be\nan object-level collision if two parties upload the exact same object, in\nwhich case it doesn't matter.\n\n\t\tM.\n"},{"id":"201266","messageId":"CAB9Jk9A8E57byg+1yzc22ByC_3VQd0j+HGu8Sj9121=LToopyg@mail.gmail.com","threadId":"31821","inReplyTo":"507C1DB4.2010000@xiplink.com","subject":"Re: push race","fromName":"Angelo Borsotti","fromEmail":"angelo.borsotti@gmail.com","sentAt":"2012-10-15T15:50:47Z","receivedAt":"2012-10-15T15:50:47Z","isPatch":false,"sender":{"key":"angelo.borsotti@gmail.com","avatar":null},"body":"Hi Marc,\n\ncorrect, there will be no file overwriting because no files are\nwritten on the work tree.\nI tried to follow the actions of the program, but did not quite catch\nthe 6. you mention.\n\n-Angelo\n"},{"id":"201281","messageId":"20121015185608.GC31658@sigill.intra.peff.net","threadId":"31821","inReplyTo":"507C1DB4.2010000@xiplink.com","subject":"Re: push race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-15T18:56:08Z","receivedAt":"2012-10-15T18:56:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 15, 2012 at 10:29:08AM -0400, Marc Branchaud wrote:\n\n> Here's a previous discussion of a race in concurrent updates to the same ref,\n> even when the updates are all identical:\n> \n> http://news.gmane.org/find-root.php?group=gmane.comp.version-control.git&article=164636\n> \n> In that thread, Peff outlines the lock procedure for refs:\n> \n> 1. get the lock\n> 2. check and remember the sha1\n> 3. release the lock\n> 4. do some long-running work (like the actual push)\n> 5. get the lock\n> 6. check that the sha1 is the same as the remembered one\n> 7. update the sha1\n> 8. release the lock\n\nA minor nit, but I was wrong on steps 1-3. We don't have to take a lock\non reading, because our write mechanism uses atomic replacement. So it\nis really:\n\n 1. read and remember the original sha1\n 2. do some long-running work (like the actual push)\n 3. get the write lock\n 4. read the sha1 and check that it's the same as our original\n 5. write the new sha1 to the lockfile\n 6. simultaneously release the lock and update the ref by atomically\n renaming the lockfile to the actual ref\n\nAny simultaneous push may see the \"old\" sha1 before step 6, and when it\ngets to its own step 4, will fail (and two processes cannot be in steps\n3-6 simultaneously).\n\n> Angelo, in your case I think one of your concurrent updates would fail in\n> step 6. As you say, this is after the changes have been uploaded. However,\n> there's none of the file-overwriting that you fear, because the changes are\n> stored in git's object database under their SHA hashes. So there'll only be\n> an object-level collision if two parties upload the exact same object, in\n> which case it doesn't matter.\n\nRight. The only thing that needs locking is the refs, because the object\ndatabase is add-only for normal operations, and by definition collisions\nmean you have the same content (or are astronomically unlucky, but your\nconsolation prize is that you can write a paper on how you found a sha1\ncollision).\n\n-Peff\n"},{"id":"201282","messageId":"20121015185802.GD31658@sigill.intra.peff.net","threadId":"31821","inReplyTo":"CAB9Jk9A8E57byg+1yzc22ByC_3VQd0j+HGu8Sj9121=LToopyg@mail.gmail.com","subject":"Re: push race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-15T18:58:02Z","receivedAt":"2012-10-15T18:58:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 15, 2012 at 05:50:47PM +0200, Angelo Borsotti wrote:\n\n> correct, there will be no file overwriting because no files are\n> written on the work tree.\n> I tried to follow the actions of the program, but did not quite catch\n> the 6. you mention.\n\nIt is the \"oldval\" parameter to refs.c:update_ref. Or if you are using\nthe \"git update-ref\" plumbing, it is the \"oldvalue\" parameter.\n\n-Peff\n"},{"id":"201304","messageId":"CAJo=hJu=eqgUhJvvpMLJ05AT6o+nVUDcm+tHV8en8OCX2-2qgA@mail.gmail.com","threadId":"31821","inReplyTo":"20121015185608.GC31658@sigill.intra.peff.net","subject":"Re: push race","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2012-10-16T02:09:52Z","receivedAt":"2012-10-16T02:09:52Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Mon, Oct 15, 2012 at 11:56 AM, Jeff King wrote:\n> Right. The only thing that needs locking is the refs, because the object\n> database is add-only for normal operations, and by definition collisions\n> mean you have the same content (or are astronomically unlucky, but your\n> consolation prize is that you can write a paper on how you found a sha1\n> collision).\n\nIts worth nothing that a SHA-1 collision can be identified at the\nserver because the server performs a byte-for-byte compare of both\ncopies of the object to make sure they match exactly in every way. Its\nnot fast, but its safe. :-)\n"},{"id":"201309","messageId":"20121016045118.GA21359@sigill.intra.peff.net","threadId":"31821","inReplyTo":"CAJo=hJu=eqgUhJvvpMLJ05AT6o+nVUDcm+tHV8en8OCX2-2qgA@mail.gmail.com","subject":"Re: push race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-16T04:51:18Z","receivedAt":"2012-10-16T04:51:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 15, 2012 at 07:09:52PM -0700, Shawn O. Pearce wrote:\n\n> On Mon, Oct 15, 2012 at 11:56 AM, Jeff King wrote:\n> > Right. The only thing that needs locking is the refs, because the object\n> > database is add-only for normal operations, and by definition collisions\n> > mean you have the same content (or are astronomically unlucky, but your\n> > consolation prize is that you can write a paper on how you found a sha1\n> > collision).\n> \n> Its worth nothing that a SHA-1 collision can be identified at the\n> server because the server performs a byte-for-byte compare of both\n> copies of the object to make sure they match exactly in every way. Its\n> not fast, but its safe. :-)\n\nDo we? I thought early versions of git did that, but we did not\ndouble-check collisions any more for performance reasons. You don't\nhappen to remember where that code is, do you (not that it really\nmatters, but I am just curious)?\n\n-Peff\n"},{"id":"201310","messageId":"CACsJy8AJVAoUHft6+rdOjWCpLWWj3m0NgvFd9pToQRQ5uD8_gg@mail.gmail.com","threadId":"31821","inReplyTo":"20121016045118.GA21359@sigill.intra.peff.net","subject":"Re: push race","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-10-16T05:15:21Z","receivedAt":"2012-10-16T05:15:21Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 16, 2012 at 11:51 AM, Jeff King wrote:\n>> Its worth nothing that a SHA-1 collision can be identified at the\n>> server because the server performs a byte-for-byte compare of both\n>> copies of the object to make sure they match exactly in every way. Its\n>> not fast, but its safe. :-)\n>\n> Do we? I thought early versions of git did that, but we did not\n> double-check collisions any more for performance reasons. You don't\n> happen to remember where that code is, do you (not that it really\n> matters, but I am just curious)?\n\nWe do. I touched that sha-1 collision code last time I updated\nindex-pack, to support large blobs. We only do that when we receive an\nobject that we already have, which should not happen often unless\nyou're under attack, so little performance impact normally. Search\n\"collision\" in index-pack.c\n-- \nDuy\n"},{"id":"201312","messageId":"20121016053750.GA22281@sigill.intra.peff.net","threadId":"31821","inReplyTo":"CACsJy8AJVAoUHft6+rdOjWCpLWWj3m0NgvFd9pToQRQ5uD8_gg@mail.gmail.com","subject":"Re: push race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-16T05:37:50Z","receivedAt":"2012-10-16T05:37:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 16, 2012 at 12:15:21PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> On Tue, Oct 16, 2012 at 11:51 AM, Jeff King wrote:\n> >> Its worth nothing that a SHA-1 collision can be identified at the\n> >> server because the server performs a byte-for-byte compare of both\n> >> copies of the object to make sure they match exactly in every way. Its\n> >> not fast, but its safe. :-)\n> >\n> > Do we? I thought early versions of git did that, but we did not\n> > double-check collisions any more for performance reasons. You don't\n> > happen to remember where that code is, do you (not that it really\n> > matters, but I am just curious)?\n> \n> We do. I touched that sha-1 collision code last time I updated\n> index-pack, to support large blobs. We only do that when we receive an\n> object that we already have, which should not happen often unless\n> you're under attack, so little performance impact normally. Search\n> \"collision\" in index-pack.c\n\nAh, thanks, I remember this now. I think that I was thinking of the very\nearly code to check every sha1 file write. E.g., the code killed off by\naac1794 (Improve sha1 object file writing., 2005-05-03). But that is\nancient history that is not really relevant.\n\nInteresting that we check only in index-pack. If the pushed content is\nsmall enough, we will call unpack-objects. That follows the usual code\npath for writing the object, which will prefer the existing copy.\n\nI suspect a site that is heavy on alternates is invoking the index-pack\ncode path more frequently than necessary (e.g., history gets pushed to\none forked repo, then when it goes to the next one, we may not share the\nref that tells the client we already have the object and receive it a\nsecond time).\n\n-Peff\n"},{"id":"201314","messageId":"CAB9Jk9A72EpMTcdVgXXWZJz-QjsAWyo1Ds5kmDqim-RtuK8b-g@mail.gmail.com","threadId":"31821","inReplyTo":"20121015185608.GC31658@sigill.intra.peff.net","subject":"Re: push race","fromName":"Angelo Borsotti","fromEmail":"angelo.borsotti@gmail.com","sentAt":"2012-10-16T06:35:53Z","receivedAt":"2012-10-16T06:35:53Z","isPatch":false,"sender":{"key":"angelo.borsotti@gmail.com","avatar":null},"body":"Hi Jeff,\n\nit would be worth to put your description as comments in the code for future\nreference.\n\nThanks\n-Angelo\n"},{"id":"201320","messageId":"CACsJy8D14sv5=+5zfiwgYCb7OoEqvQoVQ0ObAeWtUUSjRAgBeQ@mail.gmail.com","threadId":"31821","inReplyTo":"20121016053750.GA22281@sigill.intra.peff.net","subject":"Re: push race","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-10-16T10:45:12Z","receivedAt":"2012-10-16T10:45:12Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 16, 2012 at 12:37 PM, Jeff King wrote:\n> I suspect a site that is heavy on alternates is invoking the index-pack\n> code path more frequently than necessary (e.g., history gets pushed to\n> one forked repo, then when it goes to the next one, we may not share the\n> ref that tells the client we already have the object and receive it a\n> second time).\n\nI suppose we could do the way unpack-objects does: prefer present\nobjects and drop the new identical ones, no memcmp. Objects that are\nnot bases, or are ref-delta bases, can be safely dropped. ofs-delta\nbases may lead to rewriting the pack. Do-able but not sure it's worth\nthe effort.\n-- \nDuy\n"},{"id":"201357","messageId":"20121016170232.GA27243@sigill.intra.peff.net","threadId":"31821","inReplyTo":"CACsJy8D14sv5=+5zfiwgYCb7OoEqvQoVQ0ObAeWtUUSjRAgBeQ@mail.gmail.com","subject":"Re: push race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-16T17:02:33Z","receivedAt":"2012-10-16T17:02:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 16, 2012 at 05:45:12PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> On Tue, Oct 16, 2012 at 12:37 PM, Jeff King wrote:\n> > I suspect a site that is heavy on alternates is invoking the index-pack\n> > code path more frequently than necessary (e.g., history gets pushed to\n> > one forked repo, then when it goes to the next one, we may not share the\n> > ref that tells the client we already have the object and receive it a\n> > second time).\n> \n> I suppose we could do the way unpack-objects does: prefer present\n> objects and drop the new identical ones, no memcmp. Objects that are\n> not bases, or are ref-delta bases, can be safely dropped. ofs-delta\n> bases may lead to rewriting the pack. Do-able but not sure it's worth\n> the effort.\n\nYeah, I think that complexity is why we don't do it currently. We are\npretty alternates-heavy at GitHub, and we have not noticed a performance\nimpact. So I think it is probably not worth worrying about.\n\n-Peff\n"},{"id":"201360","messageId":"7vtxtus58h.fsf@alter.siamese.dyndns.org","threadId":"31821","inReplyTo":"CACsJy8D14sv5=+5zfiwgYCb7OoEqvQoVQ0ObAeWtUUSjRAgBeQ@mail.gmail.com","subject":"Re: push race","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-16T17:21:02Z","receivedAt":"2012-10-16T17:21:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Tue, Oct 16, 2012 at 12:37 PM, Jeff King wrote:\n>> I suspect a site that is heavy on alternates is invoking the index-pack\n>> code path more frequently than necessary (e.g., history gets pushed to\n>> one forked repo, then when it goes to the next one, we may not share the\n>> ref that tells the client we already have the object and receive it a\n>> second time).\n>\n> I suppose we could do the way unpack-objects does: prefer present\n> objects and drop the new identical ones, no memcmp. Objects that are\n> not bases, or are ref-delta bases, can be safely dropped. ofs-delta\n> bases may lead to rewriting the pack. Do-able but not sure it's worth\n> the effort.\n\nUntil you read all the incoming pack data, you won't know what\nobjects are used as bases for others, so unless you are keeping\neverything in core, you would have to spool the incoming data to a\nfile and then rewrite the final pack file to \"drop\" these \"can be\nsafely dropped\" objects, with or without offset delta encoding.\n"},{"id":"201366","messageId":"20121016172502.GC27243@sigill.intra.peff.net","threadId":"31821","inReplyTo":"7vtxtus58h.fsf@alter.siamese.dyndns.org","subject":"Re: push race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-16T17:25:02Z","receivedAt":"2012-10-16T17:25:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 16, 2012 at 10:21:02AM -0700, Junio C Hamano wrote:\n\n> > I suppose we could do the way unpack-objects does: prefer present\n> > objects and drop the new identical ones, no memcmp. Objects that are\n> > not bases, or are ref-delta bases, can be safely dropped. ofs-delta\n> > bases may lead to rewriting the pack. Do-able but not sure it's worth\n> > the effort.\n> \n> Until you read all the incoming pack data, you won't know what\n> objects are used as bases for others, so unless you are keeping\n> everything in core, you would have to spool the incoming data to a\n> file and then rewrite the final pack file to \"drop\" these \"can be\n> safely dropped\" objects, with or without offset delta encoding.\n\nBy definition, you know that you have another copy of these objects\n(that is why you are dropping them). So you could treat later delta\nreferences to them the same as thin-pack references, and re-add your\nexisting on-disk copy of the object to the end of the pack.\n\nBut still...the complexity is ugly, and we do not even have a measured\nproblem in the real world. This is not worth thinking about. :)\n\n-Peff\n"},{"id":"201380","messageId":"7vhapus072.fsf@alter.siamese.dyndns.org","threadId":"31821","inReplyTo":"20121016172502.GC27243@sigill.intra.peff.net","subject":"Re: push race","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-16T19:09:53Z","receivedAt":"2012-10-16T19:09:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But still...the complexity is ugly, and we do not even have a measured\n> problem in the real world. This is not worth thinking about. :)\n\nYup.\n"}]}