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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 14, 2014, 18:50 UTC
Message-ID: <xmqq7g6rpwxn.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CACsJy8C=a6a4COBdG9uLo5XNViK6Lb3+ZyWM1P_e9p7qu+shkQ@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 8 quoted lines

> On Sat, Apr 12, 2014 at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> In the spectrum between useful and insane, there is a point where we
>> should just tell the insane: don't do it then.
>
> ... A process' dying is a
> way of telling people "this is insane" without having to draw a line
> between dos and dont's in documents.

Fair enough. Instead of "that is insane and you may end up recording what you do not intend to", being able to say "that is insane and you will see some commands failing when you may end up being in such a situation" is a lot more explicit.

Thanks for injecting some sanity to me.

> Serializing write access to make both competing processes is nice, but
> that's a separate step that may or may not be worth pursuing. And I'm
> towards not worth pursuing.

I think we are in agreement on that point.

I would want to see the proposed "read without locking, compute heavily and then take a lock and revalidate before writing it out, or fail" done with a focus on not hurting the "sane" use pattern, i.e. making the "revalidate" part as light as possible. I however cannot seem to find any optimization opportunities other than the "open, read and compare the checksum". Two I thought and discarded are:

 - checking timestamp of the index file itself, and failing when it
   has changed (without opening or reading the checksum) is not
   good; it is optimizing for the wrong case, because we would need
   to check the checksum anyway when the timestamps match.

 - checking i-num of the index file itself, and failing when it has
   changed (without opening or reading the checksum) is not good,
   either.  Creating "index.lock", writing to it and renaming it to
   "index", keeping the "index" during the whole period, would
   ensure that the index file that results with this single cycle
   would get a different i-num from the original, but if that is
   done twice or more, the same i-num can be reused and defeat the
   check.