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

Re: [PATCH 00/12] Fix some reference-related races

From
Jeff King <peff@peff.net>
Date
Jun 12, 2013, 12:52 UTC
Message-ID
<20130612125235.GA28876@sigill.intra.peff.net>
In-Reply-To
<1370987312-6761-1-git-send-email-mhagger@alum.mit.edu>
On Tue, Jun 11, 2013 at 11:48:20PM +0200, Michael Haggerty wrote:
> *This patch series must be built on top of mh/reflife.*

Applying on top of what Junio has in mh/reflife seems to create conflicts at the first patch. I didn't look into it, though, but just read the patches and looked at the packed-refs-transactions branch at git://github.com/mhagger/git.git.

Show 5 quoted lines
> The other problem was that the for_each_ref() functions will die if
> the ref cache that they are iterating over is freed out from under
> them.  This problem is solved by using reference counts to avoid
> freeing the old packed ref cache (even if it is no longer valid) until
> all users are done with it.

I was worried that the reference-counting would end up invasive and ugly, but it turned out quite nice.

Overall, I think the series goes in the right direction, and I didn't see any implementation flaws. The core of the race-handling is the same as in mine, but your "hyperactive repository" patch shows how badly mine can break under load.

I sent a few comments to specific patches, but I'd be fine with applying it as-is. Thanks for working on it.

-Peff
Previous: Jeff KingNext: Ramsay Jones
Message 18 of 22 in “Fix some reference-related races”
  1. 00/12 Fix some reference-related racesMichael Haggerty, Jun 11, 2013
  2. 01/12 repack_without_ref(): split list curation and entry writingMichael Haggerty, Jun 11, 2013
  3. Jeff KingJun 12, 2013
  4. Michael HaggertyJun 12, 2013
  5. 02/12 pack_refs(): split creation of packed refs and entry writingMichael Haggerty, Jun 11, 2013
  6. 03/12 refs: wrap the packed refs cache in a level of indirectionMichael Haggerty, Jun 11, 2013
  7. 04/12 refs: implement simple transactions for the packed-refs fileMichael Haggerty, Jun 11, 2013
  8. Jeff KingJun 12, 2013
  9. 05/12 refs: manage lifetime of packed refs cache via reference countingMichael Haggerty, Jun 11, 2013
  10. 06/12 do_for_each_entry(): increment the packed refs cache refcountMichael Haggerty, Jun 11, 2013
  11. 07/12 packed_ref_cache: increment refcount when lockedMichael Haggerty, Jun 11, 2013
  12. 08/12 Extract a struct stat_data from cache_entryMichael Haggerty, Jun 11, 2013
  13. 09/12 add a stat_validity structMichael Haggerty, Jun 11, 2013
  14. 10/12 get_packed_ref_cache: reload packed-refs file when it changesMichael Haggerty, Jun 11, 2013
  15. 11/12 for_each_ref: load all loose refs before packed refsMichael Haggerty, Jun 11, 2013
  16. 12/12 refs: do not invalidate the packed-refs cache unnecessarilyMichael Haggerty, Jun 11, 2013
  17. Jeff KingJun 12, 2013
  18. Jeff KingJun 12, 2013
  19. Ramsay JonesJun 15, 2013
  20. Michael HaggertyJun 16, 2013
  21. Ramsay JonesJun 18, 2013
  22. Michael HaggertyJun 19, 2013

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.