Re: reindex concurrently and two toast indexes

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Sergei Kornilov <sk(at)zsrv(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Andreas Karlsson <andreas(at)proxel(dot)se>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: reindex concurrently and two toast indexes
Date: 2020-03-05 16:57:07
Message-ID: 20200305165707.GA35281@nol
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Mar 05, 2020 at 12:53:54PM +0900, Michael Paquier wrote:
> On Wed, Mar 04, 2020 at 09:21:45AM +0100, Julien Rouhaud wrote:
> > Thanks for the patch! I started to look at it during the weekend, but
> > I got interrupted and unfortunately didn't had time to look at it
> > since.
> No problem, thanks for looking at it. I have looked at it again this
> morning, and applied it.
> > The fix looks good to me. I also tried multiple failure scenario and
> > it's unsurprisingly working just fine. Should we add some regression
> > tests for that? I guess most of it could be borrowed from the patch
> > to fix the toast index issue I sent last week.
> I have doubts when it comes to use a strategy based on
> pg_cancel_backend() and a match of application_name (see for example
> 5ad72ce but I cannot find the associated thread). I think that we
> could design something more robust here and usable by all tests, with
> two things coming into my mind:
> - A new meta-command for isolation tests to be able to cancel a
> session with PQcancel().
> - Fault injection in the backend.
> For the case of this thread, the cancellation command would be a better
> match.

I agree that the approach wasn't quite robust. I'll try to look at adding a
new command for isolationtester, but that's probably not something we want to
put in pg13?

Here's a v3 that takes address the various comments you previously noted, and
for which I also removed the regression tests.

Note that while looking at it, I noticed another bug in RIC:

# create table t1(id integer, val text); create index on t1(val);


# reindex table concurrently t1;
^CCancel request sent
ERROR: 57014: canceling statement due to user request
LOCATION: ProcessInterrupts, postgres.c:3171

# select indexrelid::regclass, indrelid::regclass, indexrelid, indrelid from pg_index where not indisvalid;
indexrelid | indrelid | indexrelid | indrelid
t1_val_idx_ccold | t1 | 16401 | 16395
pg_toast.pg_toast_16395_index_ccold | pg_toast.pg_toast_16395 | 16400 | 16398
(2 rows)

# reindex table concurrently t1;
WARNING: 0A000: cannot reindex invalid index "public.t1_val_idx_ccold" concurrently, skipping
LOCATION: ReindexRelationConcurrently, indexcmds.c:2821
WARNING: XX002: cannot reindex invalid index "pg_toast.pg_toast_16395_index_ccold" concurrently, skipping
LOCATION: ReindexRelationConcurrently, indexcmds.c:2867

# reindex index concurrently t1_val_idx_ccold;

That case is also fixed in this patch.

Attachment Content-Type Size
0001-Don-t-reindex-invalid-indexes-on-TOAST-tables-v3.patch text/plain 5.7 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Vik Fearing 2020-03-05 17:19:59 Re: Retiring support for pre-7.3 FK constraint triggers
Previous Message Robert Haas 2020-03-05 16:55:39 Re: backup manifests