Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

On Wed, Jul 12, 2023 at 10:01:49AM -0400, Robert Haas wrote:
> I'm not sure exactly what is happening here, but it looks to me like
> ATExecReplicaIdentity() only takes ShareLock on the index and
> nevertheless feels entitled to update the pg_index tuple, which is
> pretty strange. We normally require AccessExclusiveLock to perform DDL
> on an object, and in the limited exceptions that we have to that rule
> - see AlterTableGetLockLevel - it's pretty much always a
> self-exclusive lock. Otherwise, two backends might try to do the same
> DDL operation at the same time, which would lead to low-level failures
> trying to update the same tuple such as the one seen here.
> 
> But even if that doesn't happen or is prevented by some other
> mechanism, there's still a synchronization problem. Suppose backend B1
> modifies some state via a DDL operation on table T and then afterward
> backend B2 wants to perform a non-DDL operation that depends on that
> state. Well, B1 takes some lock on the relation, and B2 takes a lock
> that would conflict with it, and that guarantees that B2 starts after
> B1 commits. That means that B2 is guaranteed to see the invalidations
> that were queued by B1, which means it will flush any state out of its
> cache that was made stale by the operation performed by B1. If the
> locks didn't conflict, B2 might start before B1 committed and either
> fail to update its caches or update them but with a version of the
> tuples that's about to be made obsolete when B1 commits. So ShareLock
> doesn't feel like a very safe choice here.

Yes, I also got to wonder whether it is OK to hold only a ShareLock
for the index being used as a replica identity.  We hold an AEL on the
parent table, and ShareLock is sufficient to prevent concurrent schema
operations until the transaction that took the lock commit.  But
surely, that feels inconsistent with the common practices in
tablecmds.c.
--
Michael