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/27/2014 08:27 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> 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,
>> ...
> 
> That is an interesting view.
> 
> How does reflog fit into the picture?  Is it a completely
> independent thing that is called from any implementation of
> ReferenceBackend interface?

That's a good question.  It certainly wouldn't work for each of the
loose and packed backends to try to do logging.  I think the best
solution would be to have a logging wrapper as explained below.

>> 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.
> 
> When an implementation of ReferenceBackend has skewed performance
> characteristics (e.g. PackedReferenceBackend really prefers to be
> modified in bulk), how would that interact with the abstraction?
> 
> For example, when the application does:
> 
>     begin_transaction()
>     for ref in many_refs():
> 	delete_reference(ref)
>     commit_transaction()
> 
> StackedReferenceBackend() that consists of Loose on top of Packed
> may want to implement the commit phase like so:
> 
>     - tell Packed backend to repack without the deleted refs
>     - tell Loose backend to delete the deleted refs

I think for any two backends that are stacked, you would need to break
down a transaction as follows (here generalized to include not only
deletions):

    packed->begin_transaction()
    loose->begin_transaction()

    # And this part is done within stacked->commit_transaction():
    for entry in many_ref_updates():
        if have_old:
            stacked->verify_reference(ref, old_sha1)

        if entry is a delete:
            packed->delete_reference(entry)
        loose->update_reference(entry)

    if (!packed->commit_transaction())
        loose->commit_transaction()

Verifying old values is impossible to do batchwise with the current API,
because the old value of the packed ref has to be verified if and only
if there is no corresponding loose ref.

> But the above would not quite work, as somebody needs to remove logs
> for refs that were only in the Packed backend, and "repack without
> these refs" API supported by the Packed backend cannot be that
> somebody---"repack packed-refs without A B C" cannot unconditionally
> remove logs for A B C without checking if A B C exists as Loose.

Correct.  That's another reason that logging has to be the
responsibility of something at the "stacked" level of abstraction or higher.

I think the logging should be done by yet another outer layer of wrapper
that only does the logging, while also passing all updates down 1:1 to
the backend that it wraps (which in our case would be a stacked
backend).  Then the loose and packed backends could remain completely
ignorant of the fact that reference updates can be logged.  I think the
logging layer could implement the same reference backend API as the
other backends.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu