NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit720b59b

committed

Avoid catalog lookups in RelationAllowsEarlyPruning().

RelationAllowsEarlyPruning() performed a catalog scan, but is usedin two contexts where that was a bad idea:1. In heap_page_prune_opt(), which runs very frequently in some large scans. This caused major performance problems in a field report that was easy to reproduce.2. In TestForOldSnapshot(), which runs while we hold a buffer content lock. It's not clear if this was guaranteed to be free of buffer deadlock risk.The check was introduced in commit2cc41ac and defended against areal problem: 9.6's hash indexes have no page LSN and so we can'tallow early pruning (ie the snapshot-too-old feature). We can removethe check from all later releases though: hash indexes are now logged,and there is no way to create UNLOGGED indexes on regular loggedtables.If a future release allows such a combination, it might need to puta similar check in place, but it'll need some more thought.Back-patch to 10.Author: Thomas MunroReviewed-by: Tom Lane, who spotted the second problemDiscussion:https://postgr.es/m/CA%2BhUKGKT8oTkp5jw_U4p0S-7UG9zsvtw_M47Y285bER6a2gD%2Bg%40mail.gmail.comDiscussion:https://postgr.es/m/CAA4eK1%2BWy%2BN4eE5zPm765h68LrkWc3Biu_8rzzi%2BOYX4j%2BiHRw%40mail.gmail.com

1 parent80d0e5b commit720b59bCopy full SHA for 720b59b

File tree

3 files changed

-44

lines changed

src
- backend/utils/cache
  - relcache.c
- include/utils
  - rel.h
  - snapmgr.h

3 files changed

-44

lines changed

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

Lines changed: 0 additions & 42 deletions

Original file line number	Diff line number	Diff line change
`@@ -5913,48 +5913,6 @@ RelationIdIsInInitFile(Oid relationId)`
`5913`	`5913`	`returnRelationSupportsSysCache(relationId);`
`5914`	`5914`	`}`
`5915`	`5915`
`5916`		`-/*`
`5917`		`- * Tells whether any index for the relation is unlogged.`
`5918`		`- *`
`5919`		`- * Note: There doesn't seem to be any way to have an unlogged index attached`
`5920`		`- * to a permanent table, but it seems best to keep this general so that it`
`5921`		`- * returns sensible results even when they seem obvious (like for an unlogged`
`5922`		`- * table) and to handle possible future unlogged indexes on permanent tables.`
`5923`		`- */`
`5924`		`-bool`
`5925`		`-RelationHasUnloggedIndex(Relationrel)`
`5926`		`-{`
`5927`		`-List*indexoidlist;`
`5928`		`-ListCell*indexoidscan;`
`5929`		`-boolresult= false;`
`5930`		`-`
`5931`		`-indexoidlist=RelationGetIndexList(rel);`
`5932`		`-`
`5933`		`-foreach(indexoidscan,indexoidlist)`
`5934`		`-{`
`5935`		`-Oidindexoid=lfirst_oid(indexoidscan);`
`5936`		`-HeapTupletp;`
`5937`		`-Form_pg_classreltup;`
`5938`		`-`
`5939`		`-tp=SearchSysCache1(RELOID,ObjectIdGetDatum(indexoid));`
`5940`		`-if (!HeapTupleIsValid(tp))`
`5941`		`-elog(ERROR,"cache lookup failed for relation %u",indexoid);`
`5942`		`-reltup= (Form_pg_class)GETSTRUCT(tp);`
`5943`		`-`
`5944`		`-if (reltup->relpersistence==RELPERSISTENCE_UNLOGGED)`
`5945`		`-result= true;`
`5946`		`-`
`5947`		`-ReleaseSysCache(tp);`
`5948`		`-`
`5949`		`-if (result== true)`
`5950`		`-break;`
`5951`		`-}`
`5952`		`-`
`5953`		`-list_free(indexoidlist);`
`5954`		`-`
`5955`		`-returnresult;`
`5956`		`-}`
`5957`		`-`
`5958`	`5916`	`/*`
`5959`	`5917`	`* Invalidate (remove) the init file during commit of a transaction that`
`5960`	`5918`	`* changed one or more of the relation cache entries that are kept in the`

`‎src/include/utils/rel.h`

Lines changed: 0 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -605,6 +605,5 @@ typedef struct ViewOptions`
`605`	`605`	`/* routines in utils/cache/relcache.c */`
`606`	`606`	`externvoidRelationIncrementReferenceCount(Relationrel);`
`607`	`607`	`externvoidRelationDecrementReferenceCount(Relationrel);`
`608`		`-externboolRelationHasUnloggedIndex(Relationrel);`
`609`	`608`
`610`	`609`	`#endif/* REL_H */`

`‎src/include/utils/snapmgr.h`

Lines changed: 0 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,6 @@`
`40`	`40`	`RelationNeedsWAL(rel) \`
`41`	`41`	`&& !IsCatalogRelation(rel) \`
`42`	`42`	`&& !RelationIsAccessibleInLogicalDecoding(rel) \`
`43`		`- && !RelationHasUnloggedIndex(rel) \`
`44`	`43`	`)`
`45`	`44`
`46`	`45`	`#defineEarlyPruningEnabled(rel) (old_snapshot_threshold >= 0 && RelationAllowsEarlyPruning(rel))`

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!

Commit720b59b

File tree

3 files changed

3 files changed

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

`‎src/include/utils/rel.h`

`‎src/include/utils/snapmgr.h`

0 commit comments