Date: Mon, 28 Oct 2019 16:14:41 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@lists.postgresql.org,
	Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
	Andreas Karlsson <andreas@proxel.se>
Subject: Re: v12.0: interrupt reindex CONCURRENTLY: ccold: ERROR:  could not
 find tuple for parent of relation ...
Message-ID: <20191028071441.GD1687@paquier.xyz>
References: <20191015164047.GA22729@telsasoft.com>
 <20191024045929.GD3661@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="SnV5plBeK2Ge1I9g"
Content-Disposition: inline
In-Reply-To: <20191024045929.GD3661@paquier.xyz>
Precedence: bulk


--SnV5plBeK2Ge1I9g
Content-Type: multipart/mixed; boundary="PPYy/fEw/8QCHSq3"
Content-Disposition: inline


--PPYy/fEw/8QCHSq3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Oct 24, 2019 at 01:59:29PM +0900, Michael Paquier wrote:
> Yes, I can confirm the report.  In the case of this scenario the
> reindex is waiting for the first transaction to finish before step 5,
> the cancellation causing the follow-up process to not be done
> (set_dead & the next ones).  So at this stage the swap has actually
> happened.  I am still analyzing the report in depths, but you don't
> have any problems with a plain index when interrupting at this stage,
> and the old index can be cleanly dropped with the new one present, so
> my first thoughts are that we are just missing some more dependency
> cleanup at the swap phase when dealing with a partition index.

Okay, I have found this one.  The issue is that at the swap phase
pg_class.relispartition of the new index is updated to use the value
of the old index (true for a partition index), however relispartition
needs to be updated as well for the old index or when trying to
interact with it we get failures as the old index is part of no
inheritance trees.  We could use just use false as the index created
concurrently is not attached to a partition with its inheritance links
updated until the swap phase, but it feels more natural to just swap
relispartition for the old and the new index, as per the attached.

This brings also the point that you could just update pg_class to fix
things if you have a broken cluster.

In short, the attached fixes the issue for me, and that's the last bug
I know of in what has been reported..
--
Michael

--PPYy/fEw/8QCHSq3
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="reindex-conc-relispartition.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 78896da391..f6c31ccd76 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1462,6 +1462,7 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexI=
d, const char *oldName)
 				newIndexTuple;
 	Form_pg_index oldIndexForm,
 				newIndexForm;
+	bool		isPartition;
 	Oid			indexConstraintOid;
 	List	   *constraintOids =3D NIL;
 	ListCell   *lc;
@@ -1491,8 +1492,10 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndex=
Id, const char *oldName)
 	namestrcpy(&newClassForm->relname, NameStr(oldClassForm->relname));
 	namestrcpy(&oldClassForm->relname, oldName);
=20
-	/* Copy partition flag to track inheritance properly */
+	/* Swap the partition flags to track inheritance properly */
+	isPartition =3D newClassForm->relispartition;
 	newClassForm->relispartition =3D oldClassForm->relispartition;
+	oldClassForm->relispartition =3D isPartition;
=20
 	CatalogTupleUpdate(pg_class, &oldClassTuple->t_self, oldClassTuple);
 	CatalogTupleUpdate(pg_class, &newClassTuple->t_self, newClassTuple);

--PPYy/fEw/8QCHSq3--

--SnV5plBeK2Ge1I9g
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl22lWEACgkQnvQgOdby
QH2RUw//UE6DDr3waHtHlaFgaJmosaFeVB/WRGn6V5MPJB3VUPWOnHrWGrZM6tS9
ji7oQob7tPGeBjV8ga83E2dx3obVSx0lRo8GBT7E+Ia6tPg6MwrTp8lSZYEh8xyQ
0WhaHY5j4PHfUiyyTb85gNkp3jLPiUoefe8GLaPbhUIcKzDrwftq+XPCR5rEXqbK
nFvcgoxNhpxlL5qTPfqU8IQdbdifTL5R7tZ4adMe2gCH8Y4laZ/lvS0thokgQSBx
6amaUvKvLnNyn6Ml0EaQsgGp7JmUXKFriWiZOC4EZ+rK3il0MXs5rdTKreG+sRe0
SihPmY2n7+xtzrZTHrqW5z587OY3SJXyVzkcRLj9PnJVfBww89ivJK+ql6oToi34
fNw1clCM30Cr6H9s3bmEmxb5K/KNRtOCJ3n7osY/KCgYBtJBYhlx8isrSt6Hlq6h
1IYPsW1FOvPoUeOW1Gdo4Fq/N6j7r2ilTifoy0fmD+0fRX1crQXK4Ak5lGFXuaNZ
MwbbwahpTg6nhcrB6PRbiHpG6ydS1qC26j7JeAE1dKMHjgvHUScGMCb7gh67q8DX
/T+10CndYpxfiTTwIawAZl70Q41QFDjLu42KYkMtN0gp1p1WJ7V3ZJams480LRgY
h7Qg85XJqljlpvOvlMkrKRCyblzVVJi+hdjWtzSSMQPXEdN3hjo=
=S0lX
-----END PGP SIGNATURE-----

--SnV5plBeK2Ge1I9g--