Re: [PATCH v4 0/6] Fix the racy split index problem
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Oct 11, 2018, 11:38 UTC
- Message-ID
- <20181011113841.GB19800@szeder.dev>
- In-Reply-To
- <87d0sgepmo.fsf@evledraar.gmail.com>
On Thu, Oct 11, 2018 at 12:36:47PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> > On Thu, Oct 11 2018, SZEDER Gábor wrote: > > > Fourth and hopefully final round of fixing occasional test failures when > > run with 'GIT_TEST_SPLIT_INDEX=yes'. The only code change is the > > extraction of a helper function to compare two cache entries' content, > > and then a couple of minor log message clarifications. The range-diff > > below is rather clear on that. > > Looks good. I'm not going to run the stress test I did on v5 on this > since the changes are just moving existing code into a fuction, unless > you'd like me to or think there's a reason to that is.
FWIW, I intend to carry this patch below and use it in tests both locally and on Travis CI. I could never trigger any of those three conditions by repeated test runs with 'GIT_TEST_SPLIT_INDEX=yes', or by deliberately constructing tricky scenarios where they might be triggered.
Perhaps with enough time I'll get lucky eventually :)
If it's not too much trouble, then perhaps you could pick it up as well? While testing previous versions of this series it turned out that your setup has much more "luck" in finding problematic timings than mine.
diff --git a/split-index.c b/split-index.c index 5820412dc5..4af535e236 100644 --- a/split-index.c +++ b/split-index.c @@ -254,8 +254,8 @@ void prepare_to_write_split_index(struct index_state *istate) continue; } if (ce->index > si->base->cache_nr) { - BUG("ce refers to a shared ce at %d, which is beyond the shared index size %d", - ce->index, si->base->cache_nr); + BUG("ce of '%s' refers to a shared ce at %d, which is beyond the shared index size %d", + ce->name, ce->index, si->base->cache_nr); } ce->ce_flags |= CE_MATCHED; /* or "shared" */ base = si->base->cache[ce->index - 1]; @@ -293,10 +293,9 @@ void prepare_to_write_split_index(struct index_state *istate) continue; } if (ce->ce_namelen != base->ce_namelen || - strcmp(ce->name, base->name)) { - ce->index = 0; - continue; - } + strcmp(ce->name, base->name)) + BUG("ce of '%s' refers to the shared ce of a different file '%s'", + ce->name, base->name); /* * This is the copy of a cache entry that is present * in the shared index, created by unpack_trees() @@ -332,7 +331,8 @@ void prepare_to_write_split_index(struct index_state *istate) * set CE_UPDATE_IN_BASE as well. */ if (compare_ce_content(ce, base)) - ce->ce_flags |= CE_UPDATE_IN_BASE; + BUG("ce of '%s' differs from its shared ce, but the CE_UPDATE_IN_BASE flag was not set", + ce->name); } discard_cache_entry(base); si->base->cache[ce->index - 1] = ce;