Re: [PATCH] Avoid open and lock the table Extendend Statistics (src/backend/commands/statscmds.c)
Tomas Vondra writes: > On 2/13/22 22:43, Andres Freund wrote: >> Having looked briefly at it, I don't understand what the locking scheme is? > I don't recall the details of the discussion (if at all), but if you try > to do concurrent ALTER STATISTICS, that'll end up with: > ERROR: tuple concurrently updated > reported by CatalogTupleUpdate. AFAIK that's what we do for other object > types that don't have a relation that we might lock (e.g. try to co > CREATE OR REPLACE FUNCTION). Yeah, we generally haven't bothered with a separate locking scheme for objects other than relations. I don't see any big problem with that for objects that are defined by a single catalog entry, since "tuple concurrently updated" failures serve fine (although maybe that error message could be nicer). Extended stats don't quite fit that definition, but at least for updates that only need to touch the pg_statistic_ext row, it's the same story. When we get around to supporting multi-relation statistics, there might need to be some protocol around when ANALYZE can update pg_statistic_ext_data rows. But I don't think that issue exists yet, since only one ANALYZE can run on a particular relation at a time. regards, tom lane
Re: [PATCH] Avoid open and lock the table Extendend Statistics (src/backend/commands/statscmds.c)
On 2/13/22 22:43, Andres Freund wrote: > Hi, > > On 2022-02-13 18:21:38 -0300, Ranier Vilela wrote: >> Why open and lock the table Extended Statistics if it is not the owner. >> Check and return to avoid this. > > I was about to say that this opens up time-to-check-time-to-use type > issues. But it's the wrong thing to lock to prevent those. > I doubt optimizing this is worth any effort - ALTER STATISTICS is rare, not doing it with owner rights is even rarer. > Having looked briefly at it, I don't understand what the locking scheme is? > Shouldn't there be at least some locking against concurrent ALTERs and between > ALTER and ANALYZE etc? > I don't recall the details of the discussion (if at all), but if you try to do concurrent ALTER STATISTICS, that'll end up with: ERROR: tuple concurrently updated reported by CatalogTupleUpdate. AFAIK that's what we do for other object types that don't have a relation that we might lock (e.g. try to co CREATE OR REPLACE FUNCTION). ANALYZE reads the statistics from the catalog, so it should see the last committed stattarget value, I think. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Avoid open and lock the table Extendend Statistics (src/backend/commands/statscmds.c)
Em dom., 13 de fev. de 2022 às 18:43, Andres Freund escreveu: > Hi, > > On 2022-02-13 18:21:38 -0300, Ranier Vilela wrote: > > Why open and lock the table Extended Statistics if it is not the owner. > > Check and return to avoid this. > > I was about to say that this opens up time-to-check-time-to-use type > issues. But it's the wrong thing to lock to prevent those. > > Having looked briefly at it, I don't understand what the locking scheme is? > Shouldn't there be at least some locking against concurrent ALTERs and > between > ALTER and ANALYZE etc? > I checked the pg_statistics_object_ownercheck function and I think the patch is wrong. It seems to me that when using SearchSysCache1, it is necessary to open and lock the table. Better remove the patch, sorry for the noise. regards, Ranier Vilela
Re: [PATCH] Avoid open and lock the table Extendend Statistics (src/backend/commands/statscmds.c)
Hi, On 2022-02-13 18:21:38 -0300, Ranier Vilela wrote: > Why open and lock the table Extended Statistics if it is not the owner. > Check and return to avoid this. I was about to say that this opens up time-to-check-time-to-use type issues. But it's the wrong thing to lock to prevent those. Having looked briefly at it, I don't understand what the locking scheme is? Shouldn't there be at least some locking against concurrent ALTERs and between ALTER and ANALYZE etc? Greetings, Andres Freund
[PATCH] Avoid open and lock the table Extendend Statistics (src/backend/commands/statscmds.c)
Hi, Commit https://github.com/postgres/postgres/commit/6d554e3fcd6fb8be2dbcbd3521e2947ed7a552cb has fixed the bug, but still has room for improvement. Why open and lock the table Extended Statistics if it is not the owner. Check and return to avoid this. trivial patch attached. regards, Ranier Vilela avoid_open_and_lock_table_if_not_owner.patch Description: Binary data
