On 07/08/2015 07:11 PM, Junio C Hamano wrote: > Michael Haggerty <mhagger@xxxxxxxxxxxx> 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@xxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html