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

On 05/15/2014 07:29 PM, Ronnie Sahlberg wrote:

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.

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.