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

[PATCH 0/2] let feature.experimental imply gc.cruftPacks=true

From
Emily Shaffer <emilyshaffer@google.com>
Date
Aug 3, 2022, 20:57 UTC
Message-ID
<20220803205721.3686361-1-emilyshaffer@google.com>

Cruft packs seem to save us disk space during garbage collection. There may still be some interesting races around mtime, but as I understand it, those are not particularly worse or better with or without cruft packs; so this setting primarily works to save us disk space. At least at Google, we see concerns around loose object explosion during gc fairly often, so this is a welcome change and we've turned it on for all Googlers. Because we did so, I wonder if it's going to make sense to for gc.cruftPacks to default to true in the future, with or without mtime race fixes. With that in mind, I think it's a good idea to get any folks who may be testing for us with feature.experimental to also try out cruft packs and complain to us if there is an issue.

In Monday's standup there was some discussion around remaining concerns with mtime and cruft packs. As I understood it at least, it seems there can be a loss of information around mtimes when switching between cruft repacks and no-cruft repacks, for example:

 ab/cdef is unreachable and has mtime <last week>
 'git gc --cruft'
 packs/<cruft>.pack contains abcdef
 packs/<cruft>.mtimes says abcdef has mtime <last week>
 'git gc' (no cruft)
 ab/cdef is unreachable and has mtime <now>

I could have misunderstood it. But it seems to be an issue primarily around switching between crufty and non-crufty gc.

Read the full conversation around mtime races: https://colabti.org/irclogger/irclogger_log/git-devel?date=2022-08-01

As for the series, it's only a two-patch series because I noticed while trying to add tests for feature.experimental => gc.cruftPacks=true that there weren't any tests around gc.cruftPacks to begin with. It's possible that there are too many tests in patch 1 - gc --cruft is tested in t5329 'expiring cruft objects with git gc' - but it looked like the content of the test was different, in that t5329's test checks to make sure the cruft pack and associated metadata are deleted during expiration, but the one I added checks that the cruft pack is generated during a 'gc --cruft' which isn't ready to expire yet.

 - Emily
Emily Shaffer (2):
  gc: add tests for --cruft and friends
  config: let feature.experimental imply gc.cruftPacks=true
 Documentation/config/feature.txt |  2 +
 builtin/gc.c                     |  6 +++
 t/t6500-gc.sh                    | 71 ++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)
-- 
2.37.1.455.g008518b4e5-goog
Next: Emily Shaffer
Message 1 of 9 in “let feature.experimental imply gc.cruftPacks=true”
  1. 0/2 let feature.experimental imply gc.cruftPacks=trueEmily Shaffer, Aug 3, 2022
  2. 1/2 gc: add tests for --cruft and friendsEmily Shaffer, Aug 3, 2022
  3. Junio C HamanoAug 3, 2022
  4. Ævar Arnfjörð BjarmasonAug 4, 2022
  5. Junio C HamanoAug 4, 2022
  6. 2/2 config: let feature.experimental imply gc.cruftPacks=trueEmily Shaffer, Aug 3, 2022
  7. Junio C HamanoAug 3, 2022
  8. Derrick StoleeAug 4, 2022
  9. Junio C HamanoAug 4, 2022

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.