From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6
Date: Thu, 02 May 2019 16:54:11 -0400
Message-ID: <10650.1556830451@sss.pgh.pa.us> (raw)
In-Reply-To: <20190502172851.klckhiyukqd3bd2t@alap3.anarazel.de>
References: <16043.1556754084@sss.pgh.pa.us>
<20190502012520.eyfhhn5ix2ooflys@alap3.anarazel.de>
<27459.1556762513@sss.pgh.pa.us>
<20190502021913.axyvyyyw5547swy5@alap3.anarazel.de>
<25661.1556808540@sss.pgh.pa.us>
<20190502153107.o5zadep4ehebzoqy@alap3.anarazel.de>
<4870.1556811688@sss.pgh.pa.us>
<20190502155612.ep5wqyninmkrz6uh@alap3.anarazel.de>
<5531.1556812964@sss.pgh.pa.us>
<2157.1556816395@sss.pgh.pa.us>
<20190502172851.klckhiyukqd3bd2t@alap3.anarazel.de>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6
In-Reply-To: <10650.1556830451@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox