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
*** a/src/backend/access/index/indexam.c
--- b/src/backend/access/index/indexam.c
***************
*** 81,87 ****
#define RELATION_CHECKS \
( \
AssertMacro(RelationIsValid(indexRelation)), \
! AssertMacro(PointerIsValid(indexRelation->rd_am)) \
)
#define SCAN_CHECKS \
--- 81,88 ----
#define RELATION_CHECKS \
( \
AssertMacro(RelationIsValid(indexRelation)), \
! AssertMacro(PointerIsValid(indexRelation->rd_am)), \
! AssertMacro(!ReindexIsProcessingIndex(RelationGetRelid(indexRelation)))
\
)
#define SCAN_CHECKS \
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
