Re: [PATCH/RFC v3 01/13] Move index v2 specific functions to their own file

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> If you found that an entry you read halfway has an inconsistent crc,
> and if you suspect that is because somebody else was writing to the
> same index, it is a _sure_ sign that you are not alone, and all the
> entries you read so far to the core, even if they weren't touched by
> that sombody else when you read them, may be stale, and worse yet,
> what you are going to read may be inconsistent with what you've read
> and have in-core (e.g. you may have read "f" before somebody else
> that is racing with you have turned it into a directory, and your
> next read may find "f/d" in the index without crc error).
>
> One sane way to avoid reading such an inconsistent state may be to
> redo this whole function, starting from the part that calls mmap().
> IOW,
>
> 	do {
> 		fd = open()
> 		mmap = xmmap(fd);
> 		close(fd);
>                 verify_various_fields(mmap);
>                 status = istate->ops->read_index(istate, mmap, mmap_size));
> 	} while (status == READ_AGAIN);
>
> I do not think the "pass fd around so that we can redo the mapping
> deep inside the callchain" is either a good idea or necessary.

By the way, you can only detect such inconsistency when you are
lucky enough that you catch the other person in the middle of
writing.

If the index you are looking at holds a large tree with very many
paths, it is possible that there are two large directories, and
after you read all entries from one, the other process starts
modifying the paths in that directory, without you ever finding it
out.  If the goal of the topic is to make the index work better in
projects with large trees, it may be wise to think about locking the
whole thing, so that you do not have to rely on the per-entry crc
and you being lucky to detect such a race.  The per-entry crc, as
far as I understand, may have been introduced primarily to detect
on-disk data corruption; it is not a suitable mechanism to detect
conflicting accesses.