Re: Broken HOT chains in system catalogs

On Fri, Apr 15, 2011 at 03:01:04PM -0400, Tom Lane wrote:
> What is happening is that the preceding "reindex table pg_index" set the
> indcheckxmin flag for pg_index's indexes.  The new table built by vacuum
> full contains no HOT chains at all, let alone broken ones, so at the end
> of reindex_index we decide we ought to clear the indcheckxmin flag.
> But we are not done swapping the old and new indexes, so the uniqueness
> check doesn't work right --- I believe it's looking into the old index
> and finding a TID that means something totally different in the new
> table.

For that matter, I believe it's also attempting to insert into the old index.

> I had originally been working on the theory that the problem was coming
> from the code that sets indcheckxmin in index_build(), but at least in
> this manifestation it's actually the code that resets the flag in
> reindex_index() that is failing.  But IMO they are both buggy as can be,
> because they are totally failing to consider the possibility that the
> table/index being hacked on is pg_index itself.  There are all sorts of
> conceivable failure modes there, such as inserting the updated tuple
> into the old copy of pg_index instead of the new one, and any of them
> might come to pass anytime someone rearranges any of the code within
> hailing distance of vacuum/reindex/etc.  I suspect that related failures
> are possible if other system catalogs need to get consulted during the
> syscache fetch, too --- if one of them is the target table, we have the
> same sort of issue of executing code that isn't expecting the catalogs
> to be moving underneath it.

I think we're safe _consulting_ the system catalogs, since systable_beginscan
notes that case and does not use obsolete indexes.  Any other system catalog
_updates_ are potentially risky, though.  Perhaps index_insert() and friends
should assert that the index is not pending rebuild?

> So, my proposal for a fix is this: change IndexBuildHeapScan so it
> never sets ii_BrokenHotChain at all if is_system_catalog is true.
> This amounts to assuming that no new indexes get added to system
> catalogs after initdb, or at least not during concurrent operations
> wherein indcheckxmin would be important.

Seems reasonable and effective.

Thanks,
nm