Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit3e3f8f2

Browse files
committed
Fix bogus cache-invalidation logic in logical replication worker.
The code recorded cache invalidation events by zeroing the "localreloid"field of affected cache entries. However, it's possible for an invalevent to occur even while we have the entry open and locked. So anill-timed inval could result in "cache lookup failed for relation 0"errors, if the worker's code tried to use the cleared field. We canfix that by creating a separate bool field to record whether the entryneeds to be revalidated. (In the back branches, cram the bool intowhat had been padding space, to avoid an ABI break in the somewhatunlikely event that any extension is looking at this struct.)Also, rearrange the logic in logicalrep_rel_open so that itdoes the right thing in cases where table_open would fail.We should retry the lookup by name in that case, but we didn't.The real-world impact of this is probably small. In the first place,the error conditions are very low probability, and in the second place,the worker would just exit and get restarted. We only noticed becausein a CLOBBER_CACHE_ALWAYS build, the failure can occur repeatedly,preventing the worker from making progress. Nonetheless, it's clearlya bug, and it impedes a useful type of testing; so back-patch to v10where this code was introduced.Discussion:https://postgr.es/m/1032727.1600096803@sss.pgh.pa.us
1 parent6e146a6 commit3e3f8f2

File tree

2 files changed

+49
-29
lines changed

2 files changed

+49
-29
lines changed

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

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include"postgres.h"
1818

19+
#include"access/relation.h"
1920
#include"access/sysattr.h"
2021
#include"access/table.h"
2122
#include"catalog/namespace.h"
@@ -77,7 +78,7 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid)
7778
{
7879
if (entry->localreloid==reloid)
7980
{
80-
entry->localreloid=InvalidOid;
81+
entry->localrelvalid=false;
8182
hash_seq_term(&status);
8283
break;
8384
}
@@ -91,7 +92,7 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid)
9192
hash_seq_init(&status,LogicalRepRelMap);
9293

9394
while ((entry= (LogicalRepRelMapEntry*)hash_seq_search(&status))!=NULL)
94-
entry->localreloid=InvalidOid;
95+
entry->localrelvalid=false;
9596
}
9697
}
9798

@@ -230,15 +231,13 @@ logicalrep_rel_att_by_name(LogicalRepRelation *remoterel, const char *attname)
230231
/*
231232
* Open the local relation associated with the remote one.
232233
*
233-
* Optionally rebuilds the Relcache mapping if it was invalidated
234-
* by local DDL.
234+
* Rebuilds the Relcache mapping if it was invalidated by local DDL.
235235
*/
236236
LogicalRepRelMapEntry*
237237
logicalrep_rel_open(LogicalRepRelIdremoteid,LOCKMODElockmode)
238238
{
239239
LogicalRepRelMapEntry*entry;
240240
boolfound;
241-
Oidrelid=InvalidOid;
242241
LogicalRepRelation*remoterel;
243242

244243
if (LogicalRepRelMap==NULL)
@@ -254,14 +253,45 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
254253

255254
remoterel=&entry->remoterel;
256255

256+
/* Ensure we don't leak a relcache refcount. */
257+
if (entry->localrel)
258+
elog(ERROR,"remote relation ID %u is already open",remoteid);
259+
257260
/*
258261
* When opening and locking a relation, pending invalidation messages are
259-
* processed which can invalidate the relation. We need to update the
260-
* local cache both when we are first time accessing the relation and when
261-
* the relation is invalidated (aka entry->localreloid is set InvalidOid).
262+
* processed which can invalidate the relation. Hence, if the entry is
263+
* currently considered valid, try to open the local relation by OID and
264+
* see if invalidation ensues.
265+
*/
266+
if (entry->localrelvalid)
267+
{
268+
entry->localrel=try_relation_open(entry->localreloid,lockmode);
269+
if (!entry->localrel)
270+
{
271+
/* Table was renamed or dropped. */
272+
entry->localrelvalid= false;
273+
}
274+
elseif (!entry->localrelvalid)
275+
{
276+
/* Note we release the no-longer-useful lock here. */
277+
table_close(entry->localrel,lockmode);
278+
entry->localrel=NULL;
279+
}
280+
}
281+
282+
/*
283+
* If the entry has been marked invalid since we last had lock on it,
284+
* re-open the local relation by name and rebuild all derived data.
262285
*/
263-
if (!OidIsValid(entry->localreloid))
286+
if (!entry->localrelvalid)
264287
{
288+
Oidrelid;
289+
intfound;
290+
Bitmapset*idkey;
291+
TupleDescdesc;
292+
MemoryContextoldctx;
293+
inti;
294+
265295
/* Try to find and lock the relation by name. */
266296
relid=RangeVarGetRelid(makeRangeVar(remoterel->nspname,
267297
remoterel->relname,-1),
@@ -272,21 +302,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
272302
errmsg("logical replication target relation \"%s.%s\" does not exist",
273303
remoterel->nspname,remoterel->relname)));
274304
entry->localrel=table_open(relid,NoLock);
275-
276-
}
277-
else
278-
{
279-
relid=entry->localreloid;
280-
entry->localrel=table_open(entry->localreloid,lockmode);
281-
}
282-
283-
if (!OidIsValid(entry->localreloid))
284-
{
285-
intfound;
286-
Bitmapset*idkey;
287-
TupleDescdesc;
288-
MemoryContextoldctx;
289-
inti;
305+
entry->localreloid=relid;
290306

291307
/* Check for supported relkind. */
292308
CheckSubscriptionRelkind(entry->localrel->rd_rel->relkind,
@@ -380,7 +396,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
380396
}
381397
}
382398

383-
entry->localreloid=relid;
399+
entry->localrelvalid=true;
384400
}
385401

386402
if (entry->state!=SUBREL_STATE_READY)
@@ -523,7 +539,7 @@ logicalrep_partmap_invalidate_cb(Datum arg, Oid reloid)
523539
{
524540
if (entry->localreloid==reloid)
525541
{
526-
entry->localreloid=InvalidOid;
542+
entry->localrelvalid=false;
527543
hash_seq_term(&status);
528544
break;
529545
}
@@ -537,7 +553,7 @@ logicalrep_partmap_invalidate_cb(Datum arg, Oid reloid)
537553
hash_seq_init(&status,LogicalRepPartMap);
538554

539555
while ((entry= (LogicalRepRelMapEntry*)hash_seq_search(&status))!=NULL)
540-
entry->localreloid=InvalidOid;
556+
entry->localrelvalid=false;
541557
}
542558
}
543559

@@ -656,6 +672,8 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
656672

657673
entry->updatable=root->updatable;
658674

675+
entry->localrelvalid= true;
676+
659677
/* state and statelsn are left set to 0. */
660678
MemoryContextSwitchTo(oldctx);
661679

‎src/include/replication/logicalrelation.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,16 @@ typedef struct LogicalRepRelMapEntry
1919
{
2020
LogicalRepRelationremoterel;/* key is remoterel.remoteid */
2121

22-
/* Mapping to local relation, filled as needed. */
22+
/* Mapping to local relation. */
2323
Oidlocalreloid;/* local relation id */
24-
Relationlocalrel;/* relcache entry */
24+
Relationlocalrel;/* relcache entry(NULL when closed)*/
2525
AttrMap*attrmap;/* map of local attributes to remote ones */
2626
boolupdatable;/* Can apply updates/deletes? */
2727

2828
/* Sync state. */
2929
charstate;
30+
/* Validity flag ... inserted here to avoid ABI break in back branches. */
31+
boollocalrelvalid;
3032
XLogRecPtrstatelsn;
3133
}LogicalRepRelMapEntry;
3234

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp