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

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().

Thanks for the test case.  That reproduces really quickly.

>> 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 do however wonder if there's other path to the problem, besides
> saving toast datums. We also release those locks early in other
> places, and while e.g. r/o locks won't cause a problem with this
> specific interlock, it could cause problem around dropping the
> relation, for example.

Hmm.  Perhaps there could be some latent issue around
toast_delete_datum()?
--
Michael