Re: Problem Observed in behavior of Create Index Concurrently and Hot Update

On 31 October 2012 08:59, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> wrote:
>
>
> On Wed, Oct 31, 2012 at 11:41 AM, Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
> wrote:
>>
>>
>>
>> According to me, the problem happens at Step-4. As at Step-4, it does the
>> HOT update due to which validate_index() is not able to put an entry for
>> C2=5
>>
>
> Thanks for the report. I can reproduce this issue. As you rightly pointed
> out, step-4 should not have been a HOT update because that would create
> broken HOT chains. We used to guard against this by consulting not-yet-ready
> or not-yet-valid indexes while deciding whether to do HOT update or not.
>
> ISTM that the following commit broke things:
>
> commit 8cb53654dbdb4c386369eb988062d0bbb6de725e
> Author: Simon Riggs <simon(at)2ndQuadrant(dot)com>
> Date: Fri Apr 6 10:21:40 2012 +0100
>
> Add DROP INDEX CONCURRENTLY [IF EXISTS], uses ShareUpdateExclusiveLock
>
> There is a comment in src/backend/commands/indexcmds.c:694
>
> /*
> * At this moment we are sure that there are no transactions with the
> * table open for write that don't have this new index in their list of
> * indexes. We have waited out all the existing transactions and any
> new
> * transaction will have the new index in its list, but the index is
> still
> * marked as "not-ready-for-inserts". The index is consulted while
> * deciding HOT-safety though. This arrangement ensures that no new HOT
> * chains can be created where the new tuple and the old tuple in the
> * chain have different index keys.
> */
>
> So we mark the index not-ready-for-inserts, but still consult it while
> deciding whether an UPDATE could be HOT or not.
>
> Somewhere else in heapam.c:2730, we do the following to get information
> about index columns for all ready/not-yet-ready indexes:
>
> hot_attrs = RelationGetIndexAttrBitmap(relation);
>
> This internally calls RelationGetIndexList() to get the list of indexes on
> the relation.
>
> The offending commit made certain changes to this function and we stopped
> returning indexes which has indisready and indisvalid set to false. This has
> caused the said regression. Since there are a bunch of callers to this
> function, I wouldn't be surprised if we see other regressions because of the
> same change.
>
> diff --git a/src/backend/utils/cache/relcache.c
> b/src/backend/utils/cache/relcache.c
> index a59950e..9cadb3f 100644
> --- a/src/backend/utils/cache/relcache.c
> +++ b/src/backend/utils/cache/relcache.c
> @@ -3355,6 +3355,12 @@ RelationGetIndexList(Relation relation)
> oidvector *indclass;
> bool isnull;
>
> + /*
> + * Ignore any indexes that are currently being dropped
> + */
> + if (!index->indisvalid && !index->indisready)
> + continue;
> +
> /* Add index's OID to result list in the proper order */
> result = insert_ordered_oid(result, index->indexrelid);

Agreed, will fix.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services