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

Commitd6d70f8

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 parentb6a5b76 commitd6d70f8

File tree

2 files changed

+44
-27
lines changed

2 files changed

+44
-27
lines changed

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

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid)
5959
{
6060
if (entry->localreloid==reloid)
6161
{
62-
entry->localreloid=InvalidOid;
62+
entry->localrelvalid=false;
6363
hash_seq_term(&status);
6464
break;
6565
}
@@ -73,7 +73,7 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid)
7373
hash_seq_init(&status,LogicalRepRelMap);
7474

7575
while ((entry= (LogicalRepRelMapEntry*)hash_seq_search(&status))!=NULL)
76-
entry->localreloid=InvalidOid;
76+
entry->localrelvalid=false;
7777
}
7878
}
7979

@@ -212,15 +212,13 @@ logicalrep_rel_att_by_name(LogicalRepRelation *remoterel, const char *attname)
212212
/*
213213
* Open the local relation associated with the remote one.
214214
*
215-
* Optionally rebuilds the Relcache mapping if it was invalidated
216-
* by local DDL.
215+
* Rebuilds the Relcache mapping if it was invalidated by local DDL.
217216
*/
218217
LogicalRepRelMapEntry*
219218
logicalrep_rel_open(LogicalRepRelIdremoteid,LOCKMODElockmode)
220219
{
221220
LogicalRepRelMapEntry*entry;
222221
boolfound;
223-
Oidrelid=InvalidOid;
224222
LogicalRepRelation*remoterel;
225223

226224
if (LogicalRepRelMap==NULL)
@@ -236,14 +234,45 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
236234

237235
remoterel=&entry->remoterel;
238236

237+
/* Ensure we don't leak a relcache refcount. */
238+
if (entry->localrel)
239+
elog(ERROR,"remote relation ID %u is already open",remoteid);
240+
239241
/*
240242
* When opening and locking a relation, pending invalidation messages are
241-
* processed which can invalidate the relation.We need to update the
242-
*local cache both when we are first time accessing the relation and when
243-
*the relation is invalidated (aka entry->localreloid is set InvalidOid).
243+
* processed which can invalidate the relation.Hence, if the entry is
244+
*currently considered valid, try to open the local relation by OID and
245+
*see if invalidation ensues.
244246
*/
245-
if (!OidIsValid(entry->localreloid))
247+
if (entry->localrelvalid)
246248
{
249+
entry->localrel=try_relation_open(entry->localreloid,lockmode);
250+
if (!entry->localrel)
251+
{
252+
/* Table was renamed or dropped. */
253+
entry->localrelvalid= false;
254+
}
255+
elseif (!entry->localrelvalid)
256+
{
257+
/* Note we release the no-longer-useful lock here. */
258+
heap_close(entry->localrel,lockmode);
259+
entry->localrel=NULL;
260+
}
261+
}
262+
263+
/*
264+
* If the entry has been marked invalid since we last had lock on it,
265+
* re-open the local relation by name and rebuild all derived data.
266+
*/
267+
if (!entry->localrelvalid)
268+
{
269+
Oidrelid;
270+
intfound;
271+
Bitmapset*idkey;
272+
TupleDescdesc;
273+
MemoryContextoldctx;
274+
inti;
275+
247276
/* Try to find and lock the relation by name. */
248277
relid=RangeVarGetRelid(makeRangeVar(remoterel->nspname,
249278
remoterel->relname,-1),
@@ -254,21 +283,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
254283
errmsg("logical replication target relation \"%s.%s\" does not exist",
255284
remoterel->nspname,remoterel->relname)));
256285
entry->localrel=heap_open(relid,NoLock);
257-
258-
}
259-
else
260-
{
261-
relid=entry->localreloid;
262-
entry->localrel=heap_open(entry->localreloid,lockmode);
263-
}
264-
265-
if (!OidIsValid(entry->localreloid))
266-
{
267-
intfound;
268-
Bitmapset*idkey;
269-
TupleDescdesc;
270-
MemoryContextoldctx;
271-
inti;
286+
entry->localreloid=relid;
272287

273288
/* Check for supported relkind. */
274289
CheckSubscriptionRelkind(entry->localrel->rd_rel->relkind,
@@ -361,7 +376,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
361376
}
362377
}
363378

364-
entry->localreloid=relid;
379+
entry->localrelvalid=true;
365380
}
366381

367382
if (entry->state!=SUBREL_STATE_READY)

‎src/include/replication/logicalrelation.h

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

21-
/* Mapping to local relation, filled as needed. */
21+
/* Mapping to local relation. */
2222
Oidlocalreloid;/* local relation id */
23-
Relationlocalrel;/* relcache entry */
23+
Relationlocalrel;/* relcache entry(NULL when closed)*/
2424
AttrNumber*attrmap;/* map of local attributes to remote ones */
2525
boolupdatable;/* Can apply updates/deletes? */
2626

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

0 commit comments

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