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

[PATCH 00/20] Delete directories left empty after ref deletion

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Feb 16, 2016, 13:22 UTC
Message-ID
<cover.1455626201.git.mhagger@alum.mit.edu>

Previously, we were pretty sloppy about leaving empty directories behind (under both $GIT_DIR/refs and $GIT_DIR/logs) when deleting references. Such directories could accumulate essentially forever. It's true that `pack-refs` deletes directories that it empties, but if a directory is *already* empty, then `pack-refs` doesn't remove it. It is also true that if an empty directory gets in the way of the creation of a *new* reference, then it is deleted. But otherwise there is no systematic cleanup of empty directories.

Aside from being messy, wasting disk space, and costing extra time when enumerating references, the proliferation of empty directories was triggering a libgit2 bug [1] on our servers. (We use temporary, uniquely-named references for internal purposes, causing a lot of empty directories to accumulate in some cases.)

This problem was also recently reported by Karl Moskowski [2] to be the cause of problem on non-case-sensitive filesystems.

This patch series makes the reference update machinery more aggressive about deleting empty directories under $GIT_DIR/refs and under $GIT_DIR/logs when deleting references and/or reflogs. This doesn't eliminate all situations where empty directories can be left behind [3], but it covers the worst offenders.

As prelude to the main change, there are a number of patches that make the *creation* of reflog directories more robust against races. Since we want to delete such directories more aggressively, we have to worry more about a race between a process that is creating a new reflog, and another process that might be deleting the containing directory at the same time. (We already had protection against this sort of race for reference creation, but not for reflog creation.)

And since I got tired of writing the same code over and over, I abstracted the code for retrying directory creation into a new function, raceproof_create_file(). This function replaces similar code that appeared in two other places and is now also used for creating reflog files. (Can anybody think of any other code that has to deal with the same kind of race and could maybe benefit?)

This patch series is also available from my GitHub account [4] as branch delete-empty-refs-dirs.

As you might imagine, this patch series conflicts with David Turner's dt/refs-backend-lmdb series [5]. But the conflicts are not too bad [6]. When one or the other of these series is ready to progress, I'd be happy to help resolve the conflicts with the other.

Michael
[1] https://github.com/libgit2/libgit2/issues/3576
[2] http://thread.gmane.org/gmane.comp.version-control.git/283504
[3] Off the top of my head, one case that is not covered is when a
    reflog is expired: a lock is acquired under $GIT_DIR/refs, but the
    directory that held it is not deleted when the lock is released.
[4] http://github.com/mhagger/git
[5] http://thread.gmane.org/gmane.comp.version-control.git/285604
[6] For a merge of this patch series with dt/refs-backend-lmdb v4, see
    branch merge-delete-empty-refs-dirs-refs-backend-lmdb at my GitHub
    repository [4].
Michael Haggerty (20):
  safe_create_leading_directories_const(): preserve errno
  safe_create_leading_directories(): set errno on SCLD_EXISTS
  raceproof_create_file(): new function
  lock_ref_sha1_basic(): use raceproof_create_file()
  rename_tmp_log(): use raceproof_create_file()
  rename_tmp_log(): improve error reporting
  log_ref_setup(): separate code for create vs non-create
  log_ref_setup(): improve robustness against races
  log_ref_setup(): pass the open file descriptor back to the caller
  log_ref_write_1(): don't depend on logfile
  log_ref_setup(): manage the name of the reflog file internally
  log_ref_write_1(): inline function
  try_remove_empty_parents(): rename parameter "name" -> "refname"
  try_remove_empty_parents(): don't trash argument contents
  try_remove_empty_parents(): don't accommodate consecutive slashes
  t5505: use "for-each-ref" to test for the non-existence of references
  delete_ref_loose(): derive loose reference path from lock
  delete_ref_loose(): inline function
  try_remove_empty_parents(): teach to remove parents of reflogs, too
  ref_transaction_commit(): clean up empty directories
 cache.h               |  26 +++-
 refs/files-backend.c  | 370 ++++++++++++++++++++++++++------------------------
 refs/refs-internal.h  |   9 +-
 sha1_file.c           |  77 ++++++++++-
 t/t1400-update-ref.sh |  27 ++++
 t/t5505-remote.sh     |   2 +-
 6 files changed, 325 insertions(+), 186 deletions(-)
-- 
2.7.0
Next: Michael Haggerty
Message 1 of 34 in “Delete directories left empty after ref deletion”
  1. 00/20 Delete directories left empty after ref deletionMichael Haggerty, Feb 16, 2016
  2. 01/20 safe_create_leading_directories_const(): preserve errnoMichael Haggerty, Feb 16, 2016
  3. Jeff KingFeb 16, 2016
  4. 02/20 safe_create_leading_directories(): set errno on SCLD_EXISTSMichael Haggerty, Feb 16, 2016
  5. Junio C HamanoFeb 17, 2016
  6. Michael HaggertyFeb 18, 2016
  7. 03/20 raceproof_create_file(): new functionMichael Haggerty, Feb 16, 2016
  8. Junio C HamanoFeb 17, 2016
  9. 04/20 lock_ref_sha1_basic(): use raceproof_create_file()Michael Haggerty, Feb 16, 2016
  10. Junio C HamanoFeb 17, 2016
  11. 05/20 rename_tmp_log(): use raceproof_create_file()Michael Haggerty, Feb 16, 2016
  12. Junio C HamanoFeb 17, 2016
  13. Michael HaggertyFeb 19, 2016
  14. Junio C HamanoFeb 19, 2016
  15. 06/20 rename_tmp_log(): improve error reportingMichael Haggerty, Feb 16, 2016
  16. Junio C HamanoFeb 18, 2016
  17. 07/20 log_ref_setup(): separate code for create vs non-createMichael Haggerty, Feb 16, 2016
  18. 08/20 log_ref_setup(): improve robustness against racesMichael Haggerty, Feb 16, 2016
  19. Junio C HamanoFeb 18, 2016
  20. 09/20 log_ref_setup(): pass the open file descriptor back to the callerMichael Haggerty, Feb 16, 2016
  21. Junio C HamanoFeb 18, 2016
  22. 10/20 log_ref_write_1(): don't depend on logfileMichael Haggerty, Feb 16, 2016
  23. 11/20 log_ref_setup(): manage the name of the reflog file internallyMichael Haggerty, Feb 16, 2016
  24. 12/20 log_ref_write_1(): inline functionMichael Haggerty, Feb 16, 2016
  25. Junio C HamanoFeb 18, 2016
  26. 13/20 try_remove_empty_parents(): rename parameter "name" -> "refname"Michael Haggerty, Feb 16, 2016
  27. 14/20 try_remove_empty_parents(): don't trash argument contentsMichael Haggerty, Feb 16, 2016
  28. 15/20 try_remove_empty_parents(): don't accommodate consecutive slashesMichael Haggerty, Feb 16, 2016
  29. 16/20 t5505: use "for-each-ref" to test for the non-existence of referencesMichael Haggerty, Feb 16, 2016
  30. 17/20 delete_ref_loose(): derive loose reference path from lockMichael Haggerty, Feb 16, 2016
  31. 18/20 delete_ref_loose(): inline functionMichael Haggerty, Feb 16, 2016
  32. 19/20 try_remove_empty_parents(): teach to remove parents of reflogs, tooMichael Haggerty, Feb 16, 2016
  33. 20/20 ref_transaction_commit(): clean up empty directoriesMichael Haggerty, Feb 16, 2016
  34. Junio C HamanoFeb 17, 2016

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.