Re: [PATCH v7 2/2] Verify index file before we opportunistically update it

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2014, 04:19 UTC
Message-ID: <7vmwfrmb6a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CACsJy8ASAvopmyhawmUkuFk4DjBd2opL1kxu_jtopTDr80BWPg@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 8 quoted lines

> On Sat, Apr 12, 2014 at 3:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Having said that, nobody sane would be running two simultaneous
>> operations that are clearly write-oriented competing with each other
>> against the same index file.
>
> When it comes to racing, sanity does not matter much. People could
> just do it without thinking what exactly is happening behind the
> scene.

Well, "a race is a race is a race" would also be my knee-jerk reaction when anybody utters the word "race", but you need to realize that we are not talking about races like object creation while "gc --auto" running in the background or two pushes trying to update the same ref to different values, which are meaningful use cases.

What is the race under discussion about? It is about the index, which corresponds one-to-one to the working tree, so in order for the "race" to matter, you need to be racing against another process that is not cooperating with you (e.g. a continuous and uncontrolled "git add" updating the index while you are doing a real work), mucking with the index in the same working tree. How could such a workflow any useful in the real life?

In the spectrum between useful and insane, there is a point where we should just tell the insane: don't do it then.

Having said that...

>> So in that sense that can be done as a less urgent follow-up for this topic.
>
> Yeah if racing at refresh time is a real problem, sure we should solve it first.

... In order to solve the write-vs-write competition in a useful way, you must serialize the competing accesses. E.g. "git add" would first take a write lock on the index before read_cache(), and then do its operation and finally release the lock by the usual write-to-lock-close-then-rename dance.

The lazy "read and refresh in-core first, hoping that we did not compete with anybody, and then check just before writing out after taking the lock" is a very good solution for the opportunistic update codepath, because it is an option not to write the result out when there actually was an update by somebody else. But such an opportunistic locking scheme does not work for write-vs-write competition. Upon a real conflict, you need to fail the entire operation.