Re: ATTACH/DETACH PARTITION CONCURRENTLY

On 2018-Nov-06, Robert Haas wrote:

> - get_partition_dispatch_recurse and ExecCreatePartitionPruneState
> both call RelationGetPartitionDesc.

My patch deals with this by caching descriptors in the active snapshot.
So those two things would get the same partition descriptor. There's no
RelationGetPartitionDesc anymore, and SnapshotGetPartitionDesc takes its
place.

(I tried to use different scoping than the active snapshot; I first
tried the Portal, then I tried the resource owner. But nothing seems to
fit as precisely as the active snapshot.)

> - expand_inherited_rtentry checks
> RelationGetPartitionDesc(oldrelation) != NULL. If so, it calls
> expand_partitioned_rtentry which fetches the same PartitionDesc again.

This can be solved by changing the test to a relkind one, as my patch
does.

> - set_relation_partition_info also calls RelationGetPartitionDesc.
> Off-hand, I think this code runs after expand_inherited_rtentry. Not
> sure what to do about this. I'm not sure what the consequences would
> be if this function and that one had different ideas about the
> partition descriptor.

Snapshot caching, like in my patch, again solves this problem.

> - tablecmds.c is pretty free about calling RelationGetPartitionDesc
> repeatedly, but it probably doesn't matter. If we're doing some kind
> of DDL that depends on the contents of the partition descriptor, we
> *had better* be holding a lock strong enough to prevent the partition
> descriptor from being changed by somebody else at the same time.

My patch deals with this by unlinking the partcache entry from the hash
table on relation invalidation, so DDL code would obtain a fresh copy
each time (lookup_partcache_entry).

In other words, I already solved these problems you list.

Maybe you could give my patch a look.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services