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

[PATCH v2 0/25] prune-safety

From
Jeff King <peff@peff.net>
Date
Oct 15, 2014, 22:32 UTC
Message-ID
<20141015223244.GA25368@peff.net>

Here's a re-roll of the patch series I posted earlier to make "git prune" keep more contiguous chunks of the object graph. The cleanups to t5304 were spun off into their own series, and are dropped here. However, the other patches seem to have multiplied in number (I must have fed them after midnight).

Here are the changes since the first round (thanks everybody for your comments):

  - fix bogus return values from freshen_file, foreach_alt_odb, and
    for_each_packed_object
  - make for_each_object_in_pack static
  - clarify commit message for "keep objects reachable from recent
    objects" patch (this was the one that confused Junio, and I
    elaborated based on our discussion)
  - clarify the definition of "loose object dirs" in the comment above
    for_each_loose_file_in_object_dir
  - in for_each_loose_file, traverse hashed loose object directories in
    numeric order, and pass the number to the subdir callback (this is
    used by prune-packed for its progress updates); as a side effect,
    this fixes the bugs Michael noticed with the subdir callback.
  - prune-packed now reuses the for_each_loose_file interface
  - use revs->ignore_missing_links so we don't barf on already-missing
    unreferenced objects
  - convert reachable.c to use traverse_commit_list instead of its own
    custom walk; this gives support for ignore_missing_links above, and
    saves us a fair bit of code.
  - while in the area, I noticed that reachable.c's reflog handling is
    the same as rev-list's --reflog option; it now builds on what's in
    revision.c.

That takes us up to patch 17. While working in reachable.c, I noticed an oddity: we consider objects in the index to be reachable during prune (which is good), but we do not when dropping them during a repack that uses --unpack-unreachable=<expiration>. The remaining patches fix that, which needed a fair bit of preparatory cleanup.

I'm really beginning to question whether the "just drop objects that are about to be pruned" optimization done in 7e52f56 (gc: do not explode objects which will be immediately pruned, 2012-04-07). It really complicates things as pack-objects and prune need to have the exact same rules (and implementing it naively, by having pack-objects run the same code as prune, is not desirable because pack-objects has _already_ done a complete expensive traversal to generate the packing list). And I fear it will get even worse if we implement some of the race-condition fixes that Michael suggested earlier.

On the other hand, the loosening behavior without 7e52f56 has some severe pathological cases. A repository which has had a chunk of history deleted can easily increase in size several orders of magnitude due to loosening (since we lose the benefit of all deltas in the loosened objects).

Finally, there are a few things that were discussed that I didn't address/fix. I don't think any of them is a critical blocker, but I did want to summarize the state:

  - when refreshing, we may update a pack's mtime multiple times. It
    probably wouldn't be too hard to cache this and only update once per
    program run, but I also don't think it's that big a deal in
    practice.
  - We will munge mtimes of objects found in alternates. If we don't
    have write access to the alternate, we'll create a local duplicate
    of the object. This is the safer thing, but I'm not sure if there
    are cases where we might try to write out a large number of objects
    which exist in an alternate (OTOH, we will eventually drop them at
    the next repack).
  - I didn't implement the "sort by inode" trick that fsck does when
    traversing the loose objects. It wouldn't be too hard, but I'm not
    convinced it's actually important.
  - I didn't convert fsck to the for_each_loose_file interface (mostly
    because I didn't do the inode-sorting trick, and while I don't think
    it matters, I didn't go to the work to show that it _doesn't_).
Here are the patches:
  [01/25]: foreach_alt_odb: propagate return value from callback
  [02/25]: isxdigit: cast input to unsigned char
  [03/25]: object_array: factor out slopbuf-freeing logic
  [04/25]: object_array: add a "clear" function
  [05/25]: clean up name allocation in prepare_revision_walk
  [06/25]: reachable: use traverse_commit_list instead of custom walk
  [07/25]: reachable: reuse revision.c "add all reflogs" code
  [08/25]: prune: factor out loose-object directory traversal
  [09/25]: reachable: mark index blobs as SEEN
  [10/25]: prune-packed: use for_each_loose_file_in_objdir
  [11/25]: count-objects: do not use xsize_t when counting object size
  [12/25]: count-objects: use for_each_loose_file_in_objdir
  [13/25]: sha1_file: add for_each iterators for loose and packed objects
  [14/25]: prune: keep objects reachable from recent objects
  [15/25]: pack-objects: refactor unpack-unreachable expiration check
  [16/25]: pack-objects: match prune logic for discarding objects
  [17/25]: write_sha1_file: freshen existing objects
  [18/25]: make add_object_array_with_context interface more sane
  [19/25]: traverse_commit_list: support pending blobs/trees with paths
  [20/25]: rev-list: document --reflog option
  [21/25]: rev-list: add --index-objects option
  [22/25]: reachable: use revision machinery's --index-objects code
  [23/25]: pack-objects: use argv_array
  [24/25]: repack: pack objects mentioned by the index
  [25/25]: pack-objects: double-check options before discarding objects
Next: Jeff King
Message 1 of 48 in “prune-safety”
  1. 0/25 prune-safetyJeff King, Oct 15, 2014
  2. 01/25 foreach_alt_odb: propagate return value from callbackJeff King, Oct 15, 2014
  3. 02/25 isxdigit: cast input to unsigned charJeff King, Oct 15, 2014
  4. Junio C HamanoOct 16, 2014
  5. 03/25 object_array: factor out slopbuf-freeing logicJeff King, Oct 15, 2014
  6. Junio C HamanoOct 16, 2014
  7. git-bundle rev handling and de-dupingJeff King, Oct 17, 2014
  8. Philip OakleyOct 17, 2014
  9. Junio C HamanoOct 17, 2014
  10. 04/25 object_array: add a "clear" functionJeff King, Oct 15, 2014
  11. 05/25 clean up name allocation in prepare_revision_walkJeff King, Oct 15, 2014
  12. 06/25 reachable: use traverse_commit_list instead of custom walkJeff King, Oct 15, 2014
  13. Junio C HamanoOct 16, 2014
  14. 07/25 reachable: reuse revision.c "add all reflogs" codeJeff King, Oct 15, 2014
  15. 08/25 prune: factor out loose-object directory traversalJeff King, Oct 15, 2014
  16. 09/25 reachable: mark index blobs as SEENJeff King, Oct 15, 2014
  17. 10/25 prune-packed: use for_each_loose_file_in_objdirJeff King, Oct 15, 2014
  18. 11/25 count-objects: do not use xsize_t when counting object sizeJeff King, Oct 15, 2014
  19. 12/25 count-objects: use for_each_loose_file_in_objdirJeff King, Oct 15, 2014
  20. 13/25 sha1_file: add for_each iterators for loose and packed objectsJeff King, Oct 15, 2014
  21. 14/25 prune: keep objects reachable from recent objectsJeff King, Oct 15, 2014
  22. 15/25 pack-objects: refactor unpack-unreachable expiration checkJeff King, Oct 15, 2014
  23. 16/25 pack-objects: match prune logic for discarding objectsJeff King, Oct 15, 2014
  24. 17/25 write_sha1_file: freshen existing objectsJeff King, Oct 15, 2014
  25. 18/25 make add_object_array_with_context interface more saneJeff King, Oct 15, 2014
  26. 19/25 traverse_commit_list: support pending blobs/trees with pathsJeff King, Oct 15, 2014
  27. 20/25 rev-list: document --reflog optionJeff King, Oct 15, 2014
  28. 21/25 rev-list: add --index-objects optionJeff King, Oct 15, 2014
  29. Junio C HamanoOct 16, 2014
  30. Jeff KingOct 17, 2014
  31. Jeff KingOct 17, 2014
  32. 22/26 rev-list: add --indexed-objects optionJeff King, Oct 17, 2014
  33. 23/26 reachable: use revision machinery's --indexed-objects codeJeff King, Oct 17, 2014
  34. 24/26 pack-objects: use argv_arrayJeff King, Oct 17, 2014
  35. 25/26 repack: pack objects mentioned by the indexJeff King, Oct 17, 2014
  36. 26/26 pack-objects: double-check options before discarding objectsJeff King, Oct 17, 2014
  37. 22/25 reachable: use revision machinery's --index-objects codeJeff King, Oct 15, 2014
  38. 23/25 pack-objects: use argv_arrayJeff King, Oct 15, 2014
  39. 24/25 repack: pack objects mentioned by the indexJeff King, Oct 15, 2014
  40. 25/25 pack-objects: double-check options before discarding objectsJeff King, Oct 15, 2014
  41. Junio C HamanoOct 16, 2014
  42. Junio C HamanoOct 16, 2014
  43. Jeff KingOct 16, 2014
  44. Jeff KingOct 16, 2014
  45. Junio C HamanoOct 16, 2014
  46. Jeff KingOct 17, 2014
  47. Jeff KingOct 18, 2014
  48. Jeff KingOct 17, 2014

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.