Date: Wed, 8 Dec 2021 11:48:37 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Noah Misch <noah@leadboat.com>, Andres Freund <andres@anarazel.de>,
	Alexey Ermakov <alexey.ermakov@dataegret.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Maxim Boguk <maxim.boguk@gmail.com>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
	Peter Geoghegan <pg@bowt.ie>
Subject: Re: BUG #17268: Possible corruption in toast index after reindex
 index concurrently
Message-ID: <YbAdBT749O7ylwGS@paquier.xyz>
References: <B706E617-2172-4460-845A-B76E5DB2E0A2@anarazel.de>
 <YYoA24hSkx6EYWpx@paquier.xyz>
 <20211109050848.oe3u6mhtomkaoxrx@alap3.anarazel.de>
 <20211109054625.GB940092@rfd.leadboat.com>
 <20211109063119.ukcvdk7uvz7ks3pa@alap3.anarazel.de>
 <20211109064217.GC940092@rfd.leadboat.com>
 <YYzd3dsYi/LiwWuc@paquier.xyz>
 <YZI+aNEnnpBASxNU@paquier.xyz>
 <Ya1tKYLozbcvw5Dw@paquier.xyz>
 <BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="UKUagYmf/LARoVv9"
Content-Disposition: inline
In-Reply-To: <BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YbAdBT749O7ylwGS%40paquier.xyz>
Precedence: bulk


--UKUagYmf/LARoVv9
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 07, 2021 at 10:04:54PM +0000, Bossart, Nathan wrote:
> I confirmed that the new tests reliably produce corruption and that
> the suggested fix resolves it.  I also lean towards the simple
> solution, but I do wonder if it creates any interesting side effects.

Thanks.

> For example, could holding the locks longer impact performance?

Do you have anything particular in mind?  It seems to me that this
boils down to the same lock taken on the parent table based on its
RTE.

> -	toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
> -	table_close(toastrel, RowExclusiveLock);
> +	toast_close_indexes(toastidxs, num_indexes, NoLock);
> +	table_close(toastrel, NoLock);
>=20
> I think it would be good to expand the comments above these changes to
> explain why we are keeping the lock.  That might help avoid similar
> problems in the future.

Yes, I have added a note, and applied the patch after looking at it
again this morning.  The test cannot be used in 12 so I have removed
it from REL_12_STABLE, as allow_system_table_mods is a PGC_POSTMASTER
there.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmGwHQUACgkQnvQgOdby
QH35kg//cmyFCeLmBkIrtE13cJ8JuGCO5LrtztQEUGKRuQA8z1JHKb1uhXourSZg
+z5NbeJ2nfBevq+C+qZWlLPOmyxyIKLdQGEqZ3VVUWvUpDdL23W4z2Rgjla461EX
Sz8b54kvErZIdREo07jDF6NOlBiXiJ6Bf7RjYshlHEBXd4Sy3mYMsEOgC4bMBvwh
aVGPxQKscRCRezgs3Avu71559UZEAcUKamnwkmX477VksRZ/py+SGp5rs1iD3E3q
OVExyIbPfcvQIzXGNyhvlzGDMf8ikyGN02HPIyOlFxNU6IDGpeTXltc6mkoPMqnk
lrziTda/W2qWPf/enIemYTbYU5Hry+l0czHRndfRVNcTkoN6JK/NfxLhOmpTnqtc
z44DY3/TE+zuybHCwAjyuoZD5uDGQOB51zEUY6HzYSa5X+LQw50XK7zzGN+MFA4e
r5GS478LZuj4yHEUkEbKOPX5eO2hJxVg+k+lA22WRjTsY6M8fK8dQ4ZFwJ5ywOUX
39+ag7CRgHmzSxuYtF/MFPNQsyvUCAVziUKC1a3cG1eZN4Ja2VXcEevPqVFv338T
9pJlSZHreNPMCdPcKkffbOuhTmQmRguRsyUP9OX9cp7eqP2NxRUQDi1jlLFb75hH
siQ1ptr/RQNud7pa4ojoArGOt+c53Z3TOWk6mbrWBd/eFwANqq0=
=3Ua9
-----END PGP SIGNATURE-----

--UKUagYmf/LARoVv9--