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
