Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6 - Mailing list pgsql-hackers

From Tom Lane
Subject Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6
Date
Msg-id 10650.1556830451@sss.pgh.pa.us
Whole thread Raw
In response to Re: REINDEX INDEX results in a crash for an index of pg_class since9.6  (Andres Freund <andres@anarazel.de>)
Responses Re: REINDEX INDEX results in a crash for an index of pg_class since9.6  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-02 12:59:55 -0400, Tom Lane wrote:
>> One interesting thing that turns up in check-world is that if wal_level
>> is minimal, we have to manually force an XID to be assigned, else
>> reindexing pg_class fails with "cannot commit a transaction that deleted
>> files but has no xid" :-(.  Perhaps there's some other cleaner place to
>> do that?

> Hm. We could replace that RecordTransactionCommit() with an xid
> assignment or such. But that seems at least as fragile. Or we could
> expand the logic we have for LogStandbyInvalidations() a few lines below
> the elog to also be able to handle files.  IIRC that was introduced to
> handle somewhat related issues about being able to run VACUUM
> (containing invalidations) without an xid.

Well, that's something we can maybe improve later.  I'm content to leave
the patch as it is for now.

>> if (RelationIsMapped(relation))
>> +    {
>> +        /* Since we're not updating pg_class, these had better not change */
>> +        Assert(classform->relfrozenxid == freezeXid);
>> +        Assert(classform->relminmxid == minmulti);
>> +        Assert(classform->relpersistence == persistence);

> Hm. Could we additionally assert that we're dealing with an index?

Will do.

>> +        /* These changes are safe even for a mapped relation */

> You'd probably have noticed that, but this one probably has to go.

Ah, right.  As I said, I'd not paid much attention to the comments yet.

I just finished a successful run of the core regression tests with CCA.
Given the calendar, I think that's about as much CCA testing as I should
do personally.  I'll make a cleanup pass on this patch and try to get it
pushed within a few hours, if there are not objections.

How do you feel about the other patch to rejigger the order of operations
in CommandCounterIncrement?  I think that's a bug, but it's probably
noncritical for most people.  What I'm leaning towards for that one is
waiting till after the minor releases, then pushing it to all branches.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: Identity columns should own only one sequence
Next
From: Andres Freund
Date:
Subject: Re: REINDEX INDEX results in a crash for an index of pg_class since9.6