NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit42750b0

committed

Ensure snapshot is registered within ScanPgRelation().

In 9.4 I added support to use a historical snapshot inScanPgRelation(), while adding logical decoding. Unfortunately aconflict with the concurrent removal of SnapshotNow was incorrectlyresolved, leading to an unregistered snapshot being used.It is not correct to use an unregistered (or non-active) snapshot foranything non-trivial, because catalog invalidations can cause thesnapshot to be invalidated.Luckily it seems unlikely to actively cause problems in practice, asScanPgRelation() requires that we already have a lock on the relation,we only look for a single row, and we don't appear to rely on theresult's tid to be correct. It however is clearly wrong and potentialnegative consequences would likely be hard to find. So it seems worthbackpatching the fix, even without a concrete hazard.Discussion:https://postgr.es/m/20200229052459.wzhqnbhrriezg4v2@alap3.anarazel.deBackpatch: 9.5-

1 parent7351bfe commit42750b0Copy full SHA for 42750b0

File tree

1 file changed

-4

lines changed

src/backend/utils/cache
- relcache.c

1 file changed

-4

lines changed

`‎src/backend/utils/cache/relcache.c`

Lines changed: 4 additions & 4 deletions

Original file line number	Diff line number	Diff line change
`@@ -313,7 +313,7 @@ ScanPgRelation(Oid targetRelId, bool indexOK, bool force_non_historic)`
`313`	`313`	`Relationpg_class_desc;`
`314`	`314`	`SysScanDescpg_class_scan;`
`315`	`315`	`ScanKeyDatakey[1];`
`316`		`-Snapshotsnapshot;`
	`316`	`+Snapshotsnapshot=NULL;`
`317`	`317`
`318`	`318`	`/*`
`319`	`319`	`* If something goes wrong during backend startup, we might find ourselves`
`@@ -343,12 +343,12 @@ ScanPgRelation(Oid targetRelId, bool indexOK, bool force_non_historic)`
`343`	`343`	`/*`
`344`	`344`	`* The caller might need a tuple that's newer than the one the historic`
`345`	`345`	`* snapshot; currently the only case requiring to do so is looking up the`
`346`		`- * relfilenode of non mapped system relations during decoding.`
	`346`	`+ * relfilenode of non mapped system relations during decoding. That`
	`347`	`+ * snapshot cant't change in the midst of a relcache build, so there's no`
	`348`	`+ * need to register the snapshot.`
`347`	`349`	`*/`
`348`	`350`	`if (force_non_historic)`
`349`	`351`	`snapshot=GetNonHistoricCatalogSnapshot(RelationRelationId);`
`350`		`-else`
`351`		`-snapshot=GetCatalogSnapshot(RelationRelationId);`
`352`	`352`
`353`	`353`	`pg_class_scan=systable_beginscan(pg_class_desc,ClassOidIndexId,`
`354`	`354`	`indexOK&&criticalRelcachesBuilt,`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit42750b0

File tree

1 file changed

1 file changed

`‎src/backend/utils/cache/relcache.c`

0 commit comments