Hi, Looking at this version of the patch now: 1) comment for "Phase 4 of REINDEX CONCURRENTLY" ends with an incomplete sentence.
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. 3) I am not sure if the swap algorithm used now actually is correct either. We have mvcc snapshots now, right, but we're still potentially taking separate snapshot for individual relcache lookups. What's stopping us from temporarily ending up with two relcache entries with the same relfilenode? Previously you swapped names - I think that might end up being easier, because having names temporarily confused isn't as bad as two indexes manipulating the same file. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
