On 6/19/17 22:54, Masahiko Sawada wrote: >> It seems to me we could just take a stronger lock around >> RemoveSubscriptionRel(), so that workers can't write in there concurrently. > > Since we reduced the lock level of updating pg_subscription_rel by > commit 521fd4795e3e the same deadlock issue will appear if we just > take a stronger lock level.
I was thinking about a more refined approach, like in the attached patch. It just changes the locking when in DropSubscription(), so that that doesn't fail if workers are doing stuff concurrently. Everything else stays the same. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From c101b6da52dd4f9f041390a937b337f20d212e5c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Tue, 20 Jun 2017 19:06:42 -0400 Subject: [PATCH] WIP Parametrize locking in RemoveSubscriptionRel --- src/backend/catalog/heap.c | 2 +- src/backend/catalog/pg_subscription.c | 6 +++--- src/backend/commands/subscriptioncmds.c | 4 ++-- src/include/catalog/pg_subscription_rel.h | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 4e5b79ef94..643e4bb1d5 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1844,7 +1844,7 @@ heap_drop_with_catalog(Oid relid) /* * Remove any associated relation synchronization states. */ - RemoveSubscriptionRel(InvalidOid, relid); + RemoveSubscriptionRel(InvalidOid, relid, RowExclusiveLock); /* * Forget any ON COMMIT action for the rel diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index c69c461b62..88b81920c8 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -369,7 +369,7 @@ GetSubscriptionRelState(Oid subid, Oid relid, XLogRecPtr *sublsn, * subscription, or for a particular relation, or both. */ void -RemoveSubscriptionRel(Oid subid, Oid relid) +RemoveSubscriptionRel(Oid subid, Oid relid, LOCKMODE lockmode) { Relation rel; HeapScanDesc scan; @@ -377,7 +377,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid) HeapTuple tup; int nkeys = 0; - rel = heap_open(SubscriptionRelRelationId, RowExclusiveLock); + rel = heap_open(SubscriptionRelRelationId, lockmode); if (OidIsValid(subid)) { @@ -405,7 +405,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid) } heap_endscan(scan); - heap_close(rel, RowExclusiveLock); + heap_close(rel, lockmode); } diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 5aae7b6f91..02cb7c47b8 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -595,7 +595,7 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data) { char *namespace; - RemoveSubscriptionRel(sub->oid, relid); + RemoveSubscriptionRel(sub->oid, relid, RowExclusiveLock); logicalrep_worker_stop(sub->oid, relid); @@ -910,7 +910,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) deleteSharedDependencyRecordsFor(SubscriptionRelationId, subid, 0); /* Remove any associated relation synchronization states. */ - RemoveSubscriptionRel(subid, InvalidOid); + RemoveSubscriptionRel(subid, InvalidOid, ExclusiveLock); /* Kill the apply worker so that the slot becomes accessible. */ logicalrep_worker_stop(subid, InvalidOid); diff --git a/src/include/catalog/pg_subscription_rel.h b/src/include/catalog/pg_subscription_rel.h index f5f6191676..b2cadb4b13 100644 --- a/src/include/catalog/pg_subscription_rel.h +++ b/src/include/catalog/pg_subscription_rel.h @@ -74,7 +74,7 @@ extern Oid SetSubscriptionRelState(Oid subid, Oid relid, char state, XLogRecPtr sublsn, bool update_only); extern char GetSubscriptionRelState(Oid subid, Oid relid, XLogRecPtr *sublsn, bool missing_ok); -extern void RemoveSubscriptionRel(Oid subid, Oid relid); +extern void RemoveSubscriptionRel(Oid subid, Oid relid, LOCKMODE lockmode); extern List *GetSubscriptionRelations(Oid subid); extern List *GetSubscriptionNotReadyRelations(Oid subid); -- 2.13.1
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers