git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv2 2/2] receive-pack: Add receive.objectCountLimit to refuse push with too many objects

From
Shawn Pearce <spearce@spearce.org>
Date
May 14, 2011, 22:27 UTC
Message-ID
<BANLkTikGPhYYGF190XqWkyc=LuJ3PwA4Jg@mail.gmail.com>
In-Reply-To
<7voc355fz9.fsf@alter.siamese.dyndns.org>
On Sat, May 14, 2011 at 10:50, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Shawn Pearce <spearce@spearce.org> writes:
>
>> If we are going to put limits in, does it make sense to try and push
>> these limits back to pack-objects in a more detailed way? You talked
>> about depth of history, or size of pack. pack-objects could
>> approximate both.
>
> I think the receiving end switches between index-pack and unpack-objects
> based on the object count,
It does...
> but if we can add a protocol extension to carry
> these estimates from the sender, we would be much better off. A large push
> with a small number of objects would want to keep everything in a pack,
> for example, but currently there is no way to do so.

Yes, my thoughts exactly. A 600 MiB push of 3 objects (1 huge blob, top level tree, commit) shouldn't be unpacked. Serving that huge blob back out to clients that want to fetch it is more efficient when its coming from a pack than from a loose object, due to the object reuse that is available from a pack. Unfortunately we don't have the data necessary to make that decision.

Probably the cleanest way to do this estimate transmission is to modify the pack stream format. Introduce a new capability for pack-stats. If advertised from receive-pack, send-pack can pass a --pack-stats flag to pack-objects. That process then writes out a PACK header with version code 4 (rather than 2), and includes additional data beyond the object count in an extended PACK header. receive-pack can then read the PACK header and handle version 2 as it does today, and version 4 by also checking the additional stat fields and dumping what is left over into either unpack-objects or index-pack like it does now. We wouldn't even need to change index-pack or unpack-objects, receive-pack could translate the PACK version 4 header into a version 2 command line flag when it invokes the sub process to handle the stream.

Hmm, or maybe this should be pack v5, just so nobody confuses it with the mythical pack v4 we haven't implemented.

If we do add this stats header to the PACK stream, we need to be a bit smarter about it. I would lay out the header with an additional 4 byte field telling us the size of the stats block, and then use repeating key-value pairs within the stats block to transmit the data. This way clients can add additional stats to the stream and receive-pack can ignore them if it doesn't understand them. We can start out with number of commits and estimated size of the packed data (based on summing up the reused object sizes) but could add more later like date of oldest commit if we later decide it would help the remote handle the pack stream, or reject it early.

As far as receive-pack rejecting the stream after seeing the header, I would like to avoid an extra round-trip between the two peers. So we probably need to modify send-pack like I suggest above to check the stream for an error message after pack-objects terminates, even if pack-objects terminates with a non-zero exit status. Even over a POSIX pipe we should have room for 512 bytes of error data to be crammed into the stream from receive-pack before we deadlock the two peers. If we wanted to be really paranoid, we would run pack-objects in the background and have send-pack constantly monitor the incoming half of the pipe for a status result. Since pack-objects is already another process this shouldn't be that difficult. (But it is harder for me in JGit where its not a separate thread.)

-- 
Shawn.
Previous: Junio C HamanoNext: Johannes Sixt
Message 51 of 53 in “receive-pack: Add receive.denyObjectLimit to refuse push with too many objects”
  1. 2/2 receive-pack: Add receive.denyObjectLimit to refuse push with too many objectsJohan Herland, May 13, 2011
  2. Junio C HamanoMay 13, 2011
  3. Johan HerlandMay 14, 2011
  4. 2/2 receive-pack: Add receive.objectCountLimit to refuse push with too many objectsJohan Herland, May 14, 2011
  5. Shawn PearceMay 14, 2011
  6. Johan HerlandMay 14, 2011
  7. Shawn PearceMay 14, 2011
  8. Johan HerlandMay 15, 2011
  9. 0/9 Push limitsJohan Herland, May 15, 2011
  10. 1/9 Update technical docs to reflect side-band-64k capability in receive-packJohan Herland, May 15, 2011
  11. 2/9 send-pack: Attempt to retrieve remote status even if pack-objects failsJohan Herland, May 15, 2011
  12. Jeff KingMay 16, 2011
  13. Jeff KingMay 16, 2011
  14. Jeff KingMay 16, 2011
  15. 1/3 connect: treat generic proxy processes like ssh processesJeff King, May 16, 2011
  16. Johannes SixtMay 16, 2011
  17. Junio C HamanoMay 16, 2011
  18. Jeff KingMay 17, 2011
  19. Johannes SixtMay 17, 2011
  20. Jeff KingMay 18, 2011
  21. 2/3 connect: let callers know if connection is a socketJeff King, May 16, 2011
  22. 3/3 send-pack: avoid deadlock on git:// push with failed pack-objectsJeff King, May 16, 2011
  23. Johannes SixtMay 16, 2011
  24. Jeff KingMay 17, 2011
  25. Windows: add a wrapper for the shutdown() system callJohannes Sixt, May 18, 2011
  26. 3/9 pack-objects: Allow --max-pack-size to be used together with --stdoutJohan Herland, May 15, 2011
  27. Shawn PearceMay 15, 2011
  28. Johan HerlandMay 16, 2011
  29. Junio C HamanoMay 16, 2011
  30. Johan HerlandMay 16, 2011
  31. 4/9 pack-objects: Teach new option --max-object-count, similar to --max-pack-sizeJohan Herland, May 15, 2011
  32. Shawn PearceMay 15, 2011
  33. Johan HerlandMay 15, 2011
  34. Shawn PearceMay 15, 2011
  35. Junio C HamanoMay 16, 2011
  36. Johan HerlandMay 16, 2011
  37. 5/9 pack-objects: Teach new option --max-commit-count, limiting #commits in packJohan Herland, May 15, 2011
  38. 6/9 receive-pack: Prepare for addition of the new 'limit-*' family of capabilitiesJohan Herland, May 15, 2011
  39. Junio C HamanoMay 16, 2011
  40. Johan HerlandMay 16, 2011
  41. Sverre RabbelierMay 16, 2011
  42. Junio C HamanoMay 16, 2011
  43. Sverre RabbelierMay 16, 2011
  44. Junio C HamanoMay 16, 2011
  45. Sverre RabbelierMay 16, 2011
  46. 7/9 send-pack/receive-pack: Allow server to refuse pushes with too many objectsJohan Herland, May 15, 2011
  47. 8/9 send-pack/receive-pack: Allow server to refuse pushing too large packsJohan Herland, May 15, 2011
  48. 9/9 send-pack/receive-pack: Allow server to refuse pushes with too many commitsJohan Herland, May 15, 2011
  49. Ævar Arnfjörð BjarmasonMay 15, 2011
  50. Junio C HamanoMay 14, 2011
  51. Shawn PearceMay 14, 2011
  52. Johannes SixtMay 13, 2011
  53. Johan HerlandMay 14, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.