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
