Re: BUG #17268: Possible corruption in toast index after reindex index concurrently

On Mon, Nov 08, 2021 at 09:08:48PM -0800, Andres Freund wrote:
> On 2021-11-09 14:02:19 +0900, Michael Paquier wrote:
> > On Mon, Nov 08, 2021 at 08:37:58PM -0800, Andres Freund wrote:
> > > On November 8, 2021 7:56:24 PM PST, Noah Misch <noah@leadboat.com> wrote:
> > >> On Mon, Nov 08, 2021 at 12:36:41PM -0800, Andres Freund wrote:
> > >>> One possible way to fix this would be to make ReindexRelationConcurrently()
> > >>> acquire a lock on the underlying table when reindexing a toast table. Another
> > >>> to not release the lock in toast_save_datum().

> > >> The latter is more future-proof.  Does it have material disadvantages?
> > > 
> > > I don't immediately see any. But I've been long of the opinion, and
> > > had plenty discussions around it, that our habit of releasing locks
> > > early is far too widely used.
> > 
> > Yes, I'd agree that not patching the reindex concurrent path would be
> > safer in the long run.  This feels a bit like e629a01, in spirit, not
> > in scope.
> 
> I wonder if we should do both...

Doing both sounds harmless.  Regarding REINDEX CONCURRENTLY on a system
catalog, I bet that can still reach bugs even if we do both, considering this:

$ git grep 'table_close.*, RowExclusiveLock' | wc -l
288