Re: [PATCH v6 1/7] refs.c: add err arguments to reflog functions

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

On 07/08/2015 07:11 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> I think your v7 of this patch goes too far, by turning a failure to
>> write to the reflog into a failure of the whole transaction. The problem
>> is that this failure comes too late, in the commit phase of the
>> transaction. Aborting at this late stage can leave some references
>> changed and others rolled back, violating the promise of atomicity.
> 
> Yeah, that sounds problematic.
> 
>> The old code reported a failure to write the reflog to stderr but didn't
>> fail the transaction. I think that behavior is more appropriate. The
>> reflog is of lower importance than the references themselves. Junio, do
>> you agree?
> 
> That is actually a loaded question.
> 
> Do I agree that the current (i.e. before this change) behaviour is
> more appropriate given the current choice of representation of refs
> and reflogs on the filesystem, treating a failure to update reflog
> as lower importance event and accept it as a limitation that it
> cannot abort the whole transaction atomically?  Compared to leaving
> the repository in a half-updated state where some refs and their
> logs are updated already, other remaining proposed updates are
> ignored, and the transaction claims to have failed even though some
> things have already changed and we cannot rollback, I would say that
> is a better compromise to treat reflog update as a lower importance.
> 
> Do I agree that reflog writing should stay to be best-effort in the
> longer term?  Not really.  If we are moving the ref API in the
> direction where we can plug a backend that is different from the
> traditional filesystem based one for storing refs, I think the
> backend should also be responsible for storing logs for the refs it
> stores, and if the backend wants to promise atomicity, then we
> should be able to fail the whole transaction when updates to refs
> could proceed but updates to the log of one of these updated refs
> cannot.  So I do not agree to cast in stone "the reflog is of lower
> importance" as a general rule.

Junio,

You make a good distinction. I was describing a compromise that we have
to make now due to the limitations of the current ref/reflog backend.
But I agree 100% that a future storage backend that can do correct
rollback of refs *and* reflogs can fail a transaction if the reflog
updates cannot be committed.

Thanks,
Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu