pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Bossart, Nathan <bossartn@amazon.com>
To: Michael Paquier <michael@paquier.xyz>
To: Noah Misch <noah@leadboat.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Alexey Ermakov <alexey.ermakov@dataegret.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Maxim Boguk <maxim.boguk@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Cc: Peter Geoghegan <pg@bowt.ie>
Subject: Re: BUG #17268: Possible corruption in toast index after reindex index concurrently
Date: Tue, 7 Dec 2021 22:04:54 +0000
Message-ID: <BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com> (raw)
In-Reply-To: <Ya1tKYLozbcvw5Dw@paquier.xyz>
References: <20211108203641.whpia3pzjofellf4@alap3.anarazel.de>
	<20211109035624.GA940092@rfd.leadboat.com>
	<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>

On 12/5/21, 5:54 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> I have been working on this one again for the last couple of days, and
> I would still go with the simple solution, releasing the row-level
> toast locks only at the end of the transaction when saving and
> deleting a toast value.  While testing, I have noticed that the
> deletion part is also important, as a REINDEX CONCURRENTLY run in
> parallel of a transaction removing a toast value would go through
> without that, rather than waiting for the transaction doing the
> deletion to commit first.  I have expanded the tests with everything I
> could think about, so I'd like to commit the attached.  The test is
> fancy with its use of allow_system_table_mods to make the toast
> relation names reliable, but it has been really useful.

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.
For example, could holding the locks longer impact performance?  (Of
course, performance is probably not a great reason to avoid a
sustainable solution.)

-	toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
-	table_close(toastrel, RowExclusiveLock);
+	toast_close_indexes(toastidxs, num_indexes, NoLock);
+	table_close(toastrel, NoLock);

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.

Nathan



view thread (28+ messages)  latest in thread

Message-ID: <BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com>
Permalink:  ../BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com/
Also on:    postgresql.org/message-id/BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com

 · 

reply

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-bugs@postgresql.org
  Cc: bossartn@amazon.com, michael@paquier.xyz, noah@leadboat.com, andres@anarazel.de, alexey.ermakov@dataegret.com, robertmhaas@gmail.com, maxim.boguk@gmail.com, pgsql-bugs@lists.postgresql.org, pg@bowt.ie
  Subject: Re: BUG #17268: Possible corruption in toast index after reindex index concurrently
  In-Reply-To: <BC0B973C-AD3A-4163-866D-9AF8F2226174@amazon.com>

* 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