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

[PATCH 00/10] Implement transactions for the packed ref store

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Aug 29, 2017, 08:20 UTC
Message-ID
<cover.1503993268.git.mhagger@alum.mit.edu>

This should be the second-to-last patch series in the quest for mmapped packed-refs.

Before this patch series, there is still more coupling than necessary between `files_ref_store` and `packed_ref_store`:

* `files_ref_store` adds packed references (e.g., during `git clone`
  and `git pack-refs`) by inserting them into the `packed_ref_store`'s
  internal `ref_cache`, then calling `commit_packed_refs()`. But
  `files_ref_store` shouldn't even *know* that `packed_ref_store` uses
  a `ref_cache`, let alone muck about in it.
* `files_ref_store` deletes packed references by calling
  `repack_without_refs()`.

Instead, implement reference transactions and `delete_refs()` for `packed_ref_store`, and change `files_ref_store` to use these standard methods rather than internal `packed_ref_store` methods.

This patch series finishes up the encapsulation of `packed_ref_store`. At the end of the series, the outside world only interacts with it via the refs API plus a couple of locking-related functions. That will make it easy for the next patch series to change the internal workings of `packed_ref_store` without affecting `files_ref_store`.

Along the way, we fix some longstanding problems with reference updates. Quoting from patch [08/10]:

    First, the old code didn't obtain the `packed-refs` lock until
    `files_transaction_finish()`. This means that a failure to acquire the
    `packed-refs` lock (e.g., due to contention with another process)
    wasn't detected until it was too late (problems like this are supposed
    to be detected in the "prepare" phase). The new code acquires the
    `packed-refs` lock in `files_transaction_prepare()`, the same stage of
    the processing when the loose reference locks are being acquired,
    removing another reason why the "prepare" phase might succeed and the
    "finish" phase might nevertheless fail.
    Second, the old code deleted the loose version of a reference before
    deleting any packed version of the same reference. This left a moment
    when another process might think that the packed version of the
    reference is current, which is incorrect. (Even worse, the packed
    version of the reference can be arbitrarily old, and might even point
    at an object that has since been garbage-collected.)
    Third, if a reference deletion fails to acquire the `packed-refs` lock
    altogether, then the old code might leave the repository in the
    incorrect state (possibly corrupt) described in the previous
    paragraph.
    Now we activate the new "packed-refs" file (sans any references that
    are being deleted) *before* deleting the corresponding loose
    references. But we hold the "packed-refs" lock until after the loose
    references have been finalized, thus preventing a simultaneous
    "pack-refs" process from packing the loose version of the reference in
    the time gap, which would otherwise defeat our attempt to delete it.

This patch series is also available as branch `packed-ref-transactions` in my fork on GitHub [1]. A draft of the patch series to change `packed_ref_store` to use mmap and get rid of its `ref_cache` is available as branch `mmap-packed-refs` from the same source.

Michael
[1] https://github.com/mhagger/git
Michael Haggerty (10):
  packed-backend: don't adjust the reference count on lock/unlock
  struct ref_transaction: add a place for backends to store data
  packed_ref_store: implement reference transactions
  packed_delete_refs(): implement method
  files_pack_refs(): use a reference transaction to write packed refs
  files_initial_transaction_commit(): use a transaction for packed refs
  t1404: demonstrate two problems with reference transactions
  files_ref_store: use a transaction to update packed refs
  packed-backend: rip out some now-unused code
  files_transaction_finish(): delete reflogs before references
 refs/files-backend.c         | 200 ++++++++++++++-----
 refs/packed-backend.c        | 457 +++++++++++++++++++++++++++++--------------
 refs/packed-backend.h        |  17 +-
 refs/refs-internal.h         |   1 +
 t/t1404-update-ref-errors.sh |  71 +++++++
 5 files changed, 534 insertions(+), 212 deletions(-)
-- 
2.14.1
Next: Michael Haggerty
Message 1 of 31 in “Implement transactions for the packed ref store”
  1. 00/10 Implement transactions for the packed ref storeMichael Haggerty, Aug 29, 2017
  2. 05/10 files_pack_refs(): use a reference transaction to write packed refsMichael Haggerty, Aug 29, 2017
  3. 04/10 packed_delete_refs(): implement methodMichael Haggerty, Aug 29, 2017
  4. Brandon WilliamsAug 29, 2017
  5. Michael HaggertyAug 30, 2017
  6. 06/10 files_initial_transaction_commit(): use a transaction for packed refsMichael Haggerty, Aug 29, 2017
  7. Jeff KingSep 8, 2017
  8. Michael HaggertySep 8, 2017
  9. 07/10 t1404: demonstrate two problems with reference transactionsMichael Haggerty, Aug 29, 2017
  10. Stefan BellerAug 30, 2017
  11. Michael HaggertyAug 31, 2017
  12. Junio C HamanoSep 8, 2017
  13. Jeff KingSep 8, 2017
  14. Michael HaggertySep 8, 2017
  15. 09/10 packed-backend: rip out some now-unused codeMichael Haggerty, Aug 29, 2017
  16. Brandon WilliamsAug 29, 2017
  17. 08/10 files_ref_store: use a transaction to update packed refsMichael Haggerty, Aug 29, 2017
  18. Jeff KingSep 8, 2017
  19. Michael HaggertySep 8, 2017
  20. Jeff KingSep 8, 2017
  21. 10/10 files_transaction_finish(): delete reflogs before referencesMichael Haggerty, Aug 29, 2017
  22. 01/10 packed-backend: don't adjust the reference count on lock/unlockMichael Haggerty, Aug 29, 2017
  23. Jeff KingSep 8, 2017
  24. Michael HaggertySep 8, 2017
  25. 02/10 struct ref_transaction: add a place for backends to store dataMichael Haggerty, Aug 29, 2017
  26. Jeff KingSep 8, 2017
  27. Michael HaggertySep 8, 2017
  28. Jeff KingSep 8, 2017
  29. 03/10 packed_ref_store: implement reference transactionsMichael Haggerty, Aug 29, 2017
  30. Brandon WilliamsAug 29, 2017
  31. Jeff KingSep 8, 2017

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.