Re: [PATCH 0/2] Fix fsmonitor after discard_index()

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 21, 2019, 14:36 UTC
Message-ID: <nycvar.QRO.7.76.6.1903211532140.41@tvgsbejvaqbjf.bet>
In-Reply-To: <87ef74a1dv.fsf@evledraar.gmail.com>

Hi Ævar,

On Mon, 18 Mar 2019, Ævar Arnfjörð Bjarmason wrote:

Show 14 quoted lines

> On Sat, Mar 16 2019, Johannes Schindelin via GitGitGadget wrote:
>
> > It was reported by Ævar Arnfjörð Bjarmason
> > [https://public-inbox.org/git/nycvar.QRO.7.76.6.1903142058130.41@tvgsbejvaqbjf.bet/T/#mb8718fe52e4721dacd3b143a09187ff9090ef4e3]
> > that there were problems with the fsmonitor feature in conjunction with the
> > newly built-in git stash/git rebase.
> >
> > The culprit really is that the fsmonitor flag that says whether it was
> > queried already was not re-set after discard_index() was called by mistake.
> >
> > This fixes that, and apparently also other long-standing fsmonitor issues.
>
> I've added this to my internal build & now the test suite passes in the
> fsmonitor mode without any test skipping.

Awesome.

Show 12 quoted lines

> > (Note that there is still a flakiness around t7519
> > [https://github.com/git-for-windows/git/pull/2127#pullrequestreview-215010574]
> > where it tries to make sure that the fsmonitor hook can prevent unnecessary
> > lstat() calls, but that seems to be unrelated to this here bug.)
>
> FWIW Since February 1st, 2018 I've run my builds on CentOS [67] through
> an GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all test and have never
> encountered this flakyness, and I built pretty much on every "next"
> push-out.
>
> The fix sounds good, just one data point on the rarity of the race in
> practice. I hadn't noticed this being flaky.

That makes me think that my bug fix may bring this racy problem to the light of day. After all, now we will query the fsmonitor a lot more times than before, and in quicker succession (because there are no slow shell commands running in between fsmonitor calls, i.e. the test suite).

I guess I will see more of that now, and maybe eventually the pain in my notification mails will be high enough that I'll allocate time to investigate.

Thank you for your feedback, though! Dscho