Re: Get stuck when dropping a subscription during synchronizing table

(was away for a while, got some time now for this again)

On 22/06/17 04:43, Peter Eisentraut wrote:
> The alternative is that we use the LockSharedObject() approach that was
> already alluded to, like in the attached patch. Both approaches would
> work equally fine AFAICT.

I agree, but I think we need bigger overhaul of the locking/management
in general. So here is patch which does much more changes.

The patch does several important things (in no particular order):
- Split SetSubscriptionRelState into AddSubscriptionRelState and
UpdateSubscriptionRelState for the reasons said upstream, it's cleaner,
there is no half-broken upsert logic and it has proper error checking
for each action.

- Do LockSharedObject in ALTER SUBSCRIPTION, DROP SUBSCRIPTION (this one
is preexisting but mentioning it for context), SetSubscriptionRelState,
AddSubscriptionRelState, and in the logicalrep_worker_launch. This means
we use granular per object locks to deal with concurrency.

- Because of above, the AccessExclusiveLock on pg_subscription is no
longer needed, just normal RowExlusiveLock is used now.

- logicalrep_worker_stop is also simplified due to the proper locking

- There is new interface logicalrep_worker_stop_at_commit which is used
by ALTER SUBSCRIPTION ... REFRESH PUBLICATION and by transactional
variant of DROP SUBSCRIPTION to only kill workers at the end of transaction.

- Locking/reading of subscription info is unified between DROP and ALTER
SUBSCRIPTION commands.

- DROP SUBSCRIPTION will kill all workers associated with subscription,
not just apply.

- The sync worker checks during startup if the relation is still subscribed.

- The sync worker will exit when waiting for apply and apply has shut-down.

- The log messages around workers and removed or disabled subscription
are now more consistent between startup and normal runtime of the worker.

- Some code deduplication and stylistic changes/simplification in
related areas.

- Fixed catcache's IndexScanOK() handling of the subscription catalog.

It's bit bigger patch but solves issues from multiple threads around
handling of ALTER/DROP subscription.

A lot of the locking that I added is normally done transparently by
dependency handling, but subscriptions and subscription relation status
do not use that much as it was deemed to bloat pg_depend needlessly
during the original patch review (it's also probably why this has
slipped through).

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services