Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Feb 13, 2015, 20:11 UTC
Message-ID: <54DE5A72.6030106@alum.mit.edu>
In-Reply-To: <xmqqa90hocc5.fsf@gitster.dls.corp.google.com>

On 02/13/2015 08:12 PM, Junio C Hamano wrote:

Show 8 quoted lines

> [...]
> As we are trying to see a way to move forward to do the right thing
> around reflog, I was wondering if locking only the symbolic ref is a
> sensible endgame.  "The right thing" being:
> 
>    When a symbolic ref S points at underlying ref R, and if R's tip
>    changes from X to Y, we would want to know from the reflog of S
>    that S used to point at X and then changed to point at Y.

Let's first talk about an ideal world if we had complete support for symbolic references.

Yes, I agree with your principle. Moreover, suppose that S and S2 *both* point at R, and there is a third symref S3 that points at symref S (symbolic refs can point at other symbolic refs!):

X <- R <- S <- S3
        \
         S2

Now, if R updated from X to Y (regardless of whether the update is via R directly or via one of the symrefs), then each of the four reflogs (R, S, S2, and S3) should gain a new entry reflecting the update.

If S is reseated to point at R2 instead of R, then the reflogs for S and for S3 should each gain new entries

What locks should we hold? In my opinion, we should hold the locks on exactly those references (symbolic or regular) whose reflogs we want to change. So in the first example, we should hold

    $GIT_DIR/$R.lock
    $GIT_DIR/$S.lock
    $GIT_DIR/$S2.lock, and
    $GIT_DIR/$S3.lock

Ideally, we should acquire all of the locks before making any modifications.

Now back to the real world. Currently, if R is changed *through* a symbolic reference S, then the reflogs for both R and S are updated, but not the reflogs for any other symbolic references that might point at R. If R is changed directly, then no symref's reflogs are affected, except for the special case that HEAD's reflog is changed if it points directly at R. This limitation is a hack to avoid having to walk symrefs backwards to find any symrefs that might be pointing at R.

If a symref is reseated, then its reflog is changed but not that of any symrefs that might be pointed at it.

It might actually not be extremely expensive to follow symrefs backwards. Symbolic references cannot be packed, so we would only have to scan the loose references; we could ignore packed refs. But it would still be a lot more expensive than just updating one file. I don't know that it's worth it, given that symbolic references are used so sparingly.

I think that the rule about locks as expressed above can be carried over the the real world:

    We should hold the locks on exactly those references (symbolic
    or regular) whose reflogs we plan to change. We should acquire all
    of the locks before making any changes.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu