On 2023-Jul-25, Michael Paquier wrote:
> On Mon, Jul 24, 2023 at 07:40:04PM +0000, Imseih (AWS), Sami wrote:
> > WaitForOlderSnapshots is used here to ensure that snapshots older than
> > the start of the ALTER TABLE DETACH CONCURRENTLY are completely removed
> > to guarantee consistency, however it does seem to cause deadlocks for at
> > least RR/SERIALIZABLE transactions.
> >
> > There are cases [2] in which certain operations are accepted as not being
> > MVCC-safe, and now I am wondering if this is another case? Deadlocks are
> > not a good scenario, and WaitForOlderSnapshot does not appear to do
> > anything for READ COMMITTED transactions.
> >
> > So do we actually need WaitForOlderSnapshot in the FINALIZE code? and
> > Could be acceptable that this operation is marked as not MVCC-safe like
> > the other aforementioned operations?
>
> I guess that there is an argument with lifting that a bit. Based on
> the test case your are providing, a deadlock occuring between the
> FINALIZE and a scan of the top-level partitioned table in a
> transaction that began before the DETACH CONCURRENTLY does not seem
> like the best answer to have from the user perspective. I got to
> wonder whether there is room to make the wait for older snapshots in
> the finalize phase more robust, though, and actually make it wait
> until the first transaction commits rather than fail because of a
> deadlock like that.
It took a lot of work to get SERIALIZABLE/RR transactions to work with
DETACHED CONCURRENTLY, so I'm certainly not in a hurry to give up and
just "document that it doesn't work". I don't think that would be an
acceptable feature regression.
I spent some time yesterday trying to turn the reproducer into an
isolationtester spec file. I have not succeeded yet, but I'll continue
with that this afternoon.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Every machine is a smoke machine if you operate it wrong enough."
https://twitter.com/libseybieda/status/1541673325781196801