Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit cd142e0

Browse filesBrowse files
author
Amit Kapila
committed
Make pg_replication_origin_drop safe against concurrent drops.
Currently, we get the origin id from the name and then drop the origin by taking ExclusiveLock on ReplicationOriginRelationId. So, two concurrent sessions can get the id from the name at the same time and then when they try to drop the origin, one of the sessions will get the either "tuple concurrently deleted" or "cache lookup failed for replication origin ..". To prevent this race condition we do the entire operation under lock. This obviates the need for replorigin_drop() API and we have removed it so if any extension authors are using it they need to instead use replorigin_drop_by_name. See it's usage in pg_replication_origin_drop(). Author: Peter Smith Reviewed-by: Amit Kapila, Euler Taveira, Petr Jelinek, and Alvaro Herrera Discussion: https://www.postgresql.org/message-id/CAHut%2BPuW8DWV5fskkMWWMqzt-x7RPcNQOtJQBp6SdwyRghCk7A%40mail.gmail.com
1 parent 31c7fb4 commit cd142e0
Copy full SHA for cd142e0

File tree

Expand file treeCollapse file tree

3 files changed

+38
-28
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+38
-28
lines changed

‎src/backend/commands/subscriptioncmds.c

Copy file name to clipboardExpand all lines: src/backend/commands/subscriptioncmds.c
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
926926
ListCell *lc;
927927
char originname[NAMEDATALEN];
928928
char *err = NULL;
929-
RepOriginId originid;
930929
WalReceiverConn *wrconn = NULL;
931930
StringInfoData cmd;
932931
Form_pg_subscription form;
@@ -1050,9 +1049,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
10501049

10511050
/* Remove the origin tracking if exists. */
10521051
snprintf(originname, sizeof(originname), "pg_%u", subid);
1053-
originid = replorigin_by_name(originname, true);
1054-
if (originid != InvalidRepOriginId)
1055-
replorigin_drop(originid, false);
1052+
replorigin_drop_by_name(originname, true, false);
10561053

10571054
/*
10581055
* If there is no slot associated with the subscription, we can finish

‎src/backend/replication/logical/origin.c

Copy file name to clipboardExpand all lines: src/backend/replication/logical/origin.c
+36-23Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -322,27 +322,15 @@ replorigin_create(char *roname)
322322
return roident;
323323
}
324324

325-
326325
/*
327-
* Drop replication origin.
328-
*
329-
* Needs to be called in a transaction.
326+
* Helper function to drop a replication origin.
330327
*/
331-
void
332-
replorigin_drop(RepOriginId roident, bool nowait)
328+
static void
329+
replorigin_drop_guts(Relation rel, RepOriginId roident, bool nowait)
333330
{
334331
HeapTuple tuple;
335-
Relation rel;
336332
int i;
337333

338-
Assert(IsTransactionState());
339-
340-
/*
341-
* To interlock against concurrent drops, we hold ExclusiveLock on
342-
* pg_replication_origin throughout this function.
343-
*/
344-
rel = table_open(ReplicationOriginRelationId, ExclusiveLock);
345-
346334
/*
347335
* First, clean up the slot state info, if there is any matching slot.
348336
*/
@@ -415,11 +403,40 @@ replorigin_drop(RepOriginId roident, bool nowait)
415403
ReleaseSysCache(tuple);
416404

417405
CommandCounterIncrement();
418-
419-
/* now release lock again */
420-
table_close(rel, ExclusiveLock);
421406
}
422407

408+
/*
409+
* Drop replication origin (by name).
410+
*
411+
* Needs to be called in a transaction.
412+
*/
413+
void
414+
replorigin_drop_by_name(char *name, bool missing_ok, bool nowait)
415+
{
416+
RepOriginId roident;
417+
Relation rel;
418+
419+
Assert(IsTransactionState());
420+
421+
/*
422+
* To interlock against concurrent drops, we hold ExclusiveLock on
423+
* pg_replication_origin till xact commit.
424+
*
425+
* XXX We can optimize this by acquiring the lock on a specific origin by
426+
* using LockSharedObject if required. However, for that, we first to
427+
* acquire a lock on ReplicationOriginRelationId, get the origin_id, lock
428+
* the specific origin and then re-check if the origin still exists.
429+
*/
430+
rel = table_open(ReplicationOriginRelationId, ExclusiveLock);
431+
432+
roident = replorigin_by_name(name, missing_ok);
433+
434+
if (OidIsValid(roident))
435+
replorigin_drop_guts(rel, roident, nowait);
436+
437+
/* We keep the lock on pg_replication_origin until commit */
438+
table_close(rel, NoLock);
439+
}
423440

424441
/*
425442
* Lookup replication origin via its oid and return the name.
@@ -1256,16 +1273,12 @@ Datum
12561273
pg_replication_origin_drop(PG_FUNCTION_ARGS)
12571274
{
12581275
char *name;
1259-
RepOriginId roident;
12601276

12611277
replorigin_check_prerequisites(false, false);
12621278

12631279
name = text_to_cstring((text *) DatumGetPointer(PG_GETARG_DATUM(0)));
12641280

1265-
roident = replorigin_by_name(name, false);
1266-
Assert(OidIsValid(roident));
1267-
1268-
replorigin_drop(roident, true);
1281+
replorigin_drop_by_name(name, false, true);
12691282

12701283
pfree(name);
12711284

‎src/include/replication/origin.h

Copy file name to clipboardExpand all lines: src/include/replication/origin.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ extern PGDLLIMPORT TimestampTz replorigin_session_origin_timestamp;
4040
/* API for querying & manipulating replication origins */
4141
extern RepOriginId replorigin_by_name(char *name, bool missing_ok);
4242
extern RepOriginId replorigin_create(char *name);
43-
extern void replorigin_drop(RepOriginId roident, bool nowait);
43+
extern void replorigin_drop_by_name(char *name, bool missing_ok, bool nowait);
4444
extern bool replorigin_by_oid(RepOriginId roident, bool missing_ok,
4545
char **roname);
4646

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.