git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: "Duy Nguyen" <pclouds@gmail.com>, git <git@vger.kernel.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH v3 14/21] read-cache: touch shared index files when used
Date: Tue, 31 Jan 2017 13:09:11 -0800	[thread overview]
Message-ID: <xmqqbmunq6mg.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqy3xrqbkr.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Tue, 31 Jan 2017 11:22:12 -0800")

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

> Yes, but you need to realize that "it is better not to bother users
> with a report of failure to touch in read-only repository" and "we
> ignore all failures".

Sorry about an unfinished sentence here.  "need to realize that
... and ... are different things."

> ... It is very similar
> to a situation where you ... run "status".
> The command first runs the equivalent of "update-index --refresh"
> only in-core, and it attempts to write the updated index because
> (1) it paid the cost of doing the refreshing already, and (2) if it
> can write into the index, it will help future operations in the
> repository.  But it does not report a failure to write the index
> exactly because it is merely doing an opportunistic write.
>
> And in the "we read from the split index, and we attempt to touch
> the underlying shared one to update its timestamp" case, it is OK
> not to report if we failed to touch.
> ...
> ... On the
> other hand, if you added new information, i.e. wrote the split index
> based on it, it is a good indication that the <split,shared> index
> pair has information that is more valuable.  We must warn in that
> case.

This reminds us of a third case.  What should happen if we are doing
the "opportunistic writing of the index" in "git status", managed to
write the split index, but failed to touch the shared one?

In the ideal world, I think we should do the following sequence:

 - "status" tries to write cache to the file.

 - we try to write and on any error, we return error to the caller,
   who is already prepared to ignore it and stay silent.

    - as the first step of writing the index, we first try to touch
      the shared one.  If it fails, we return an error here without
      writing the split one out.

    - then we try to write the split one out.  If this fails, we
      also return an error.

    - otherwise, both touching of the shared one and writing of the
      split one are successful.  

 - "status" finishes the opportunistic refreshing of the index, by
   either ignoring an error silently (if either touching of shared
   one or writing of split one fails) or writing the refreshed index
   out successfully.

It is OK to swap the order of touching the shared one and writing of
the split one in the above sequence, as long as an error in either
step signals a failure to the opportunistic caller.

I do not offhand know if the split-index code is structured in such
a way to allow the above sequence easily, or it needs refactoring.

If such a restructuring is required, it might not be within the
scope of the series and I am OK if you just left the NEEDSWORK
comment that describes the above (i.e. what we should be doing) as
an in-code comment so that we can pick it up later.  The whole point
of the step 14/21 on the other hand is to make sure that a shared
index that is still in active use will not go stale, and from that
point of view, such a "punting" may not be a good idea---it
deliberately finishes the series knowing that it does not adequately
do what it promises to do.  

So, ... I dunno.


  reply	other threads:[~2017-01-31 21:39 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-26 10:22 [PATCH v3 00/21] Add configuration options for split-index Christian Couder
2016-12-26 10:22 ` [PATCH v3 01/21] config: mark an error message up for translation Christian Couder
2016-12-26 10:22 ` [PATCH v3 02/21] config: add git_config_get_split_index() Christian Couder
2016-12-26 10:22 ` [PATCH v3 03/21] split-index: add {add,remove}_split_index() functions Christian Couder
2016-12-26 10:22 ` [PATCH v3 04/21] read-cache: add and then use tweak_split_index() Christian Couder
2016-12-26 10:22 ` [PATCH v3 05/21] update-index: warn in case of split-index incoherency Christian Couder
2016-12-26 10:22 ` [PATCH v3 06/21] t1700: add tests for core.splitIndex Christian Couder
2016-12-27 19:04   ` Junio C Hamano
2017-01-02  8:29     ` Christian Couder
2017-01-03 12:58       ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 07/21] Documentation/config: add information " Christian Couder
2016-12-26 10:22 ` [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var Christian Couder
2016-12-27 19:07   ` Junio C Hamano
2017-01-02  9:39     ` Christian Couder
2017-01-07 21:38       ` Junio C Hamano
2017-01-09 11:18         ` Duy Nguyen
2017-01-23 15:55           ` Christian Couder
2017-01-23 19:59             ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 09/21] config: add git_config_get_max_percent_split_change() Christian Couder
2016-12-26 10:22 ` [PATCH v3 10/21] read-cache: regenerate shared index if necessary Christian Couder
2016-12-27 19:08   ` Junio C Hamano
2017-01-02 11:23     ` Christian Couder
2016-12-26 10:22 ` [PATCH v3 11/21] t1700: add tests for splitIndex.maxPercentChange Christian Couder
2016-12-26 10:22 ` [PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange Christian Couder
2016-12-27 19:09   ` Junio C Hamano
2017-01-02 13:50     ` Christian Couder
2016-12-26 10:22 ` [PATCH v3 13/21] sha1_file: make check_and_freshen_file() non static Christian Couder
2016-12-27 19:09   ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 14/21] read-cache: touch shared index files when used Christian Couder
2016-12-27 19:10   ` Junio C Hamano
2017-01-02 14:09     ` Christian Couder
2017-01-07 21:46       ` Junio C Hamano
2017-01-09 10:55         ` Duy Nguyen
2017-01-09 14:34           ` Junio C Hamano
2017-01-19 12:13             ` Duy Nguyen
2017-01-19 19:00               ` Junio C Hamano
2017-01-20 10:44                 ` Duy Nguyen
2017-01-23 18:14                 ` Christian Couder
2017-01-23 18:53                   ` Junio C Hamano
2017-01-25 14:35                     ` Christian Couder
2017-01-25 20:52                       ` Junio C Hamano
2017-01-31 14:06                         ` Christian Couder
2017-01-31 19:22                           ` Junio C Hamano
2017-01-31 21:09                             ` Junio C Hamano [this message]
2016-12-26 10:22 ` [PATCH v3 15/21] config: add git_config_get_expiry() from gc.c Christian Couder
2016-12-26 10:22 ` [PATCH v3 16/21] read-cache: unlink old sharedindex files Christian Couder
2016-12-26 10:22 ` [PATCH v3 17/21] t1700: test shared index file expiration Christian Couder
2016-12-26 10:22 ` [PATCH v3 18/21] read-cache: refactor read_index_from() Christian Couder
2016-12-26 10:22 ` [PATCH v3 19/21] read-cache: use freshen_shared_index() in read_index_from() Christian Couder
2016-12-26 10:22 ` [PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire Christian Couder
2016-12-27 19:11   ` Junio C Hamano
2017-01-02 14:33     ` Christian Couder
2016-12-26 10:22 ` [PATCH v3 21/21] Documentation/git-update-index: explain splitIndex.* Christian Couder
2016-12-27 19:13   ` Junio C Hamano
2017-01-02 16:04     ` Christian Couder
2016-12-26 10:29 ` [PATCH v3 00/21] Add configuration options for split-index Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbmunq6mg.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).