On Wed, Feb 05, 2025 at 08:45:06PM -0800, Jeff Davis wrote: > v45-0001 addresses this by locking both the partitioned index, as well > as its table, in ShareUpdateExclusive mode. That satisfies the in-place > update requirement to take a ShareUpdateExclusiveLock on the > partitioned index, while otherwise being the same as normal indexes > (and therefore unlikely to cause a problem if ANALYZE sets stats on > partitioned indexes in the future). > > That means: > * For indexes: ShareUpdateExclusiveLock on table and AccessShareLock > on index > * For partitioned indexes: ShareUpdateExclusiveLock on table and > ShareUpdateExclusiveLock on index > * Otherwise, ShareupdateExclusiveLock on the relation > > which makes sense to me. The v45-0001 patch itself could use some > cleanup, but I can take care of that at commit time if we agree on the > locking scheme.
Fine by me. The regression tests of v45 and v46 are a bit fuzzy regarding the tests around locking for partitioned tables. For example, with v46 applied on top of HEAD, if I manipulate the internals of the patch in stats_check_arg_pair() so as we don't take a lock on the parent table, then a make check is still happy and passes even if the internals are clearly broken. I would recommend to work a bit more the tests by updating the stats of a relation with the SQL functions in a transaction and add some queries on pg_locks for locktype = 'relation' that are able to check the locks we are taking when running these operations (return pairs of relation::regclass and mode, for example). Doing that for non-partitioned relations is also something I would do, so as the locking schema we are using is clearly tracked and that future manipulations of the area would help one in tracking problems. Bonus points: scans of pg_locks are cheap tests. -- Michael
signature.asc
Description: PGP signature
