Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only

[Michael Haggerty <mhagger@alum.mit.edu>]

On 05/15/2014 07:29 PM, Ronnie Sahlberg wrote:
> Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete.
> This flag indicates that the ref does not exist as a loose ref andf only as
> a packed ref. If this is the case we then change the commit code so that
> we skip taking out a lock file and we skip calling delete_ref_loose.
> Check for this flag and die(BUG:...) if used with _update or _create.
> 
> At the start of the transaction, before we even start locking any refs,
> we add all such REF_ISPACKONLY refs to delnames so that we have a list of
> all pack only refs that we will be deleting during this transaction.

Let me make a few comments about how I think ref packing fits into the
scheme for pluggable reference back ends.  The comments may or may not
be relevant to this particular commit.

We want to have a reference API that can be implemented by various back
ends.  Obviously we need operations to read a reference, enumerate
existing references, update references within a transaction, and
probably do similar things with symbolic references.

The status quo is that we have a single reference back end consisting of
loose references sitting on top of packed references.

But really, loose references and packed references are two relatively
independent reference back ends [1].  We just happen to use them layered
on top of each other.

This suggests to me that our current structure is best modeled as two
independent reference back ends, with a third implementation of the same
reference API whose job it is to compose the first two.  In pseudocode,

    interface ReferenceBackend:
        read_ref(refname)
        __iter__(...)
        begin_transaction(...)
        update_reference(...)
        ...
        commit_transaction(...)

    class LooseReferenceBackend(ReferenceBackend):
        ...

    class PackedReferenceBackend(ReferenceBackend):
        ...

    class StackedReferenceBackend(ReferenceBackend):
        def __init__(self, backend1, backend2):
            self.backend1 = backend1
            self.backend2 = backend2

        def read_ref(refname):
            try:
                return backend1.read_ref(refname)
            except ReferenceNotFound:
                return backend2.read_ref(refname)

        def __iter__(...):
            i1 = self.backend1.iterate()
            i2 = self.backend2.iterate()
            while i1.has_next() and i2.has_next():
                if i1.peek_next() < i2.peek_next():
                    yield i1.next()
                elif i1.peek_next() == i2.peek_next():
                    yield i1.next()
                    i2.next() # discard
                else
                    yield i2.next()
            while i1.has_next():
                yield i1.next()
            while i2.has_next():
                yield i2.next()

        def pack_refs(...):
            ...

et cetera.

>From this point of view it is clear that packing refs is not an
operation that belongs in the ReferenceBackend API, but rather in the
StackedReferenceBackend interface.  The reason is that it doesn't affect
how references appear to the outside world, but rather just reorganizes
them between the two backends to which StackedReferenceBackend
delegates.  The *implementation* of pack_refs() involves adding
references to backend2 and deleting them from backend1.  But since
adding and deleting references *are* part of the ReferenceBackend API,
StackedReferenceBackend can compose *any arbitrary two* reference
backends (including other StackedReferenceBackends).  Pruning references
that have been packed is also not part of the ReferenceBackend API.
Rather it is a plain old deletion of a reference from backend1 of a
StackedReferenceBackend.

I haven't thought through all of the ramifications of this design, but
I'm pretty sure it's where we want to be headed.

Michael

[1] Forget for the sake of this discussion that we can't store symbolic
references as packed refs.

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/