Re: Index corruption with CREATE INDEX CONCURRENTLY

On Mon, Feb 6, 2017 at 10:28 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
>> Hmm. Consider that the first time relcahe invalidation occurs while
>> computing id_attrs, so now the retry logic will compute the correct
>> set of attrs (considering two indexes, if we take the example given by
>> Alvaro above.). However, the attrs computed for hot_* and key_* will
>> be using only one index, so this will lead to a situation where two of
>> the attrs (hot_attrs and key_attrs) are computed with one index and
>> id_attrs is computed with two indexes. I think this can lead to
>> Assertion in HeapSatisfiesHOTandKeyUpdate().
>
> That seems like something we'd be best off to fix by changing the
> assertion.
>

The above-mentioned problem doesn't exist in your version of the patch
(which is now committed) because the index attrs are cached after
invalidation and I have mentioned the same in my yesterday's followup
mail. The guarantee of safety is that we don't handle invalidation
between two consecutive calls to RelationGetIndexAttrBitmap() in
heap_update, but if we do handle due to any reason which is not
apparent to me, then I think there is some chance of hitting the
assertion.

> I doubt it's really going to be practical to guarantee that
> those bitmapsets are always 100% consistent throughout a transaction.
>

Agreed. As the code stands, I think it is only expected to be
guaranteed across three consecutive calls to
RelationGetIndexAttrBitmap() in heap_update. Now, if the assertion in
HeapSatisfiesHOTandKeyUpdate() is useless and we can remove it, then
probably we don't need a guarantee to be consistent in three
consecutive calls as well.

>> Okay, please find attached patch which is an extension of Tom's and
>> Andres's patch and I think it fixes the above problem noted by me.
>
> I don't like this patch at all. It only fixes the above issue if the
> relcache inval arrives while RelationGetIndexAttrBitmap is executing.
> If the inval arrives between two such calls, you still have the problem
> of the second one delivering a bitmapset that might be inconsistent
> with the first one.
>

I think it won't happen between two consecutive calls to
RelationGetIndexAttrBitmap in heap_update. It can happen during
multi-row update, but that should be fine. I think this version of
patch only defers the creation of fresh bitmapsets where ever
possible.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com