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

Re: [PATCH 5/5] reflog expire: don't assert the OID when locking refs

From
Jeff King <peff@peff.net>
Date
Mar 14, 2019, 00:44 UTC
Message-ID
<20190314004409.GJ31968@sigill.intra.peff.net>
In-Reply-To
<20190313235439.30439-6-avarab@gmail.com>
On Thu, Mar 14, 2019 at 12:54:39AM +0100, Ævar Arnfjörð Bjarmason wrote:
> The locking protocol for reflogs involves getting a *.lock file on the
> loose ref[1] (even if the actual ref is packed). This isn't needed to
> expire the reflog, and needlessly results promotes reference update
> contention to hard errors in e.g. "gc".

This first paragraph threw me off. It sounds like you are saying we don't need to take a lock, but we absolutely do. It's just that we don't need to care about the lock having some particular value. Which you do go on to explain, but I think it would be more clear by simply removing this first paragraph.

> If we instead lock the reference without considering what OID we last
> saw it at, we won't encounter user-visible contention to the extent
> that core.filesRefLockTimeout mitigates it. See 4ff0f01cb7 ("refs:
> retry acquiring reference locks for 100ms", 2017-08-21).

I think this part is true. I'd love to get a confirmation from Michael Haggerty, who has spent way more time thinking about ref and reflog locking than any mortal should have to. Hopefully he even still remembers some of it. :)

-Peff
Previous: Ævar Arnfjörð BjarmasonNext: Jeff King
Message 58 of 60 in “BUG: Race condition due to reflog expiry in "gc"”
  1. Ævar Arnfjörð BjarmasonMar 12, 2019
  2. Junio C HamanoMar 13, 2019
  3. Ævar Arnfjörð BjarmasonMar 13, 2019
  4. Jeff KingMar 13, 2019
  5. Ævar Arnfjörð BjarmasonMar 13, 2019
  6. 0/5 gc: minor code cleanup + contention fixesÆvar Arnfjörð Bjarmason, Mar 13, 2019
  7. 0/7 gc: minor code cleanup + contention fixesÆvar Arnfjörð Bjarmason, Mar 14, 2019
  8. 0/8 gc: minor code cleanup + contention fixesÆvar Arnfjörð Bjarmason, Mar 15, 2019
  9. Junio C HamanoMar 18, 2019
  10. 0/7 gc: tests and handle reflog expire configÆvar Arnfjörð Bjarmason, Mar 28, 2019
  11. 1/7 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 28, 2019
  12. 2/7 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 28, 2019
  13. 3/7 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 28, 2019
  14. 5/7 reflog tests: test for the "points nowhere" warningÆvar Arnfjörð Bjarmason, Mar 28, 2019
  15. 4/7 reflog tests: make use of "test_config" idiomÆvar Arnfjörð Bjarmason, Mar 28, 2019
  16. 7/7 gc: handle & check gc.reflogExpire configÆvar Arnfjörð Bjarmason, Mar 28, 2019
  17. 6/7 reflog tests: assert lack of early exit with expiry="never"Ævar Arnfjörð Bjarmason, Mar 28, 2019
  18. 1/8 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 15, 2019
  19. 2/8 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 15, 2019
  20. 3/8 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 15, 2019
  21. 4/8 reflog tests: make use of "test_config" idiomÆvar Arnfjörð Bjarmason, Mar 15, 2019
  22. 5/8 reflog tests: test for the "points nowhere" warningÆvar Arnfjörð Bjarmason, Mar 15, 2019
  23. 6/8 reflog tests: assert lack of early exit with expiry="never"Ævar Arnfjörð Bjarmason, Mar 15, 2019
  24. Jeff KingMar 19, 2019
  25. 7/8 gc: handle & check gc.reflogExpire configÆvar Arnfjörð Bjarmason, Mar 15, 2019
  26. 8/8 reflog expire: don't assert the OID when locking refsÆvar Arnfjörð Bjarmason, Mar 15, 2019
  27. Jeff KingMar 19, 2019
  28. 1/7 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 14, 2019
  29. 2/7 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 14, 2019
  30. Duy NguyenMar 15, 2019
  31. Ævar Arnfjörð BjarmasonMar 15, 2019
  32. Johannes SchindelinMar 15, 2019
  33. 3/7 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 14, 2019
  34. 4/7 reflog tests: make use of "test_config" idiomÆvar Arnfjörð Bjarmason, Mar 14, 2019
  35. 5/7 reflog: exit early if there's no work to doÆvar Arnfjörð Bjarmason, Mar 14, 2019
  36. Duy NguyenMar 15, 2019
  37. Ævar Arnfjörð BjarmasonMar 15, 2019
  38. 7/7 reflog expire: don't assert the OID when locking refsÆvar Arnfjörð Bjarmason, Mar 14, 2019
  39. Duy NguyenMar 15, 2019
  40. Ævar Arnfjörð BjarmasonMar 15, 2019
  41. 6/7 gc: don't run "reflog expire" when keeping reflogsÆvar Arnfjörð Bjarmason, Mar 14, 2019
  42. Duy NguyenMar 15, 2019
  43. Ævar Arnfjörð BjarmasonMar 15, 2019
  44. Duy NguyenMar 15, 2019
  45. Johannes SchindelinMar 15, 2019
  46. Junio C HamanoMar 18, 2019
  47. 1/5 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 13, 2019
  48. Jeff KingMar 14, 2019
  49. 2/5 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 13, 2019
  50. Jeff KingMar 14, 2019
  51. 3/5 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 13, 2019
  52. Jeff KingMar 14, 2019
  53. 4/5 gc: don't run "reflog expire" when keeping reflogsÆvar Arnfjörð Bjarmason, Mar 13, 2019
  54. Jeff KingMar 14, 2019
  55. Junio C HamanoMar 14, 2019
  56. Jeff KingMar 14, 2019
  57. 5/5 reflog expire: don't assert the OID when locking refsÆvar Arnfjörð Bjarmason, Mar 13, 2019
  58. Jeff KingMar 14, 2019
  59. Jeff KingMar 14, 2019
  60. Ævar Arnfjörð BjarmasonMar 21, 2019

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.