Re: Support for REINDEX CONCURRENTLY

On Thu, Sep 26, 2013 at 7:34 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2013-09-26 12:13:30 +0900, Michael Paquier wrote:
>> > 2) I don't think the drop algorithm used now is correct. Your
>> > index_concurrent_set_dead() sets both indisvalid = false and indislive =
>> > false at the same time. It does so after doing a WaitForVirtualLocks() -
>> > but that's not sufficient. Between waiting and setting indisvalid =
>> > false another transaction could start which then would start using that
>> > index. Which will not get updated anymore by other concurrent backends
>> > because of inislive = false.
>> > You really need to follow index_drop's lead here and first unset
>> > indisvalid then wait till nobody can use the index for querying anymore
>> > and only then unset indislive.
>
>> Sorry, I do not follow you here. index_concurrent_set_dead calls
>> index_set_state_flags that sets indislive and *indisready* to false,
>> not indisvalid. The concurrent index never uses indisvalid = true so
>> it can never be called by another backend for a read query. The drop
>> algorithm is made to be consistent with DROP INDEX CONCURRENTLY btw.
>
> That makes it even worse... You can do the concurrent drop only in the
> following steps:
> 1) set indisvalid = false, no future relcache lookups will have it as valid
indisvalid is never set to true for the concurrent index. Swap is done
with concurrent index having indisvalid = false and former index with
indisvalid = true. The concurrent index is validated with
index_validate in a transaction before swap transaction.
-- 
Michael