Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f4d7f1a

Browse files
committed
Fix incorrect order of operations during sinval reset processing.
We have to be sure that we have revalidated each nailed-in-cache relcache entry before we try to use it to load data for some other relcache entry. The introduction of "mapped relations" in 9.0 broke this, because although we updated the state kept in relmapper.c early enough, we failed to propagate that information into relcache entries soon enough; in particular, we could try to fetch pg_class rows out of pg_class before we'd updated its relcache entry's rd_node.relNode value from the map. This bug accounts for Dave Gould's report of failures after "vacuum full pg_class", and I believe that there is risk for other system catalogs as well. The core part of the fix is to copy relmapper data into the relcache entries during "phase 1" in RelationCacheInvalidate(), before they'll be used in "phase 2". To try to future-proof the code against other similar bugs, I also rearranged the order in which nailed relations are visited during phase 2: now it's pg_class first, then pg_class_oid_index, then other nailed relations. This should ensure that RelationClearRelation can apply RelationReloadIndexInfo to all nailed indexes without risking use of not-yet-revalidated relcache entries. Back-patch to 9.0 where the relation mapper was introduced.
1 parent 7b0d0e9 commit f4d7f1a

File tree

1 file changed

+34
-23
lines changed

1 file changed

+34
-23
lines changed

src/backend/utils/cache/relcache.c

+34-23
Original file line numberDiff line numberDiff line change
@@ -2104,11 +2104,11 @@ RelationCacheInvalidateEntry(Oid relationId)
21042104
* so hash_seq_search will complete safely; (b) during the second pass we
21052105
* only hold onto pointers to nondeletable entries.
21062106
*
2107-
* The two-phase approach also makes it easy to ensure that we process
2108-
* nailed-in-cache indexes before other nondeletable items, and that we
2109-
* process pg_class_oid_index first of all. In scenarios where a nailed
2110-
* index has been given a new relfilenode, we have to detect that update
2111-
* before the nailed index is used in reloading any other relcache entry.
2107+
* The two-phase approach also makes it easy to update relfilenodes for
2108+
* mapped relations before we do anything else, and to ensure that the
2109+
* second pass processes nailed-in-cache items before other nondeletable
2110+
* items. This should ensure that system catalogs are up to date before
2111+
* we attempt to use them to reload information about other open relations.
21122112
*/
21132113
void
21142114
RelationCacheInvalidate(void)
@@ -2120,6 +2120,11 @@ RelationCacheInvalidate(void)
21202120
List *rebuildList = NIL;
21212121
ListCell *l;
21222122

2123+
/*
2124+
* Reload relation mapping data before starting to reconstruct cache.
2125+
*/
2126+
RelationMapInvalidateAll();
2127+
21232128
/* Phase 1 */
21242129
hash_seq_init(&status, RelationIdCache);
21252130

@@ -2130,7 +2135,7 @@ RelationCacheInvalidate(void)
21302135
/* Must close all smgr references to avoid leaving dangling ptrs */
21312136
RelationCloseSmgr(relation);
21322137

2133-
/* Ignore new relations, since they are never SI targets */
2138+
/* Ignore new relations, since they are never cross-backend targets */
21342139
if (relation->rd_createSubid != InvalidSubTransactionId)
21352140
continue;
21362141

@@ -2144,22 +2149,33 @@ RelationCacheInvalidate(void)
21442149
}
21452150
else
21462151
{
2152+
/*
2153+
* If it's a mapped relation, immediately update its rd_node in
2154+
* case its relfilenode changed. We must do this during phase 1
2155+
* in case the relation is consulted during rebuild of other
2156+
* relcache entries in phase 2. It's safe since consulting the
2157+
* map doesn't involve any access to relcache entries.
2158+
*/
2159+
if (RelationIsMapped(relation))
2160+
RelationInitPhysicalAddr(relation);
2161+
21472162
/*
21482163
* Add this entry to list of stuff to rebuild in second pass.
2149-
* pg_class_oid_index goes on the front of rebuildFirstList, other
2150-
* nailed indexes on the back, and everything else into
2151-
* rebuildList (in no particular order).
2164+
* pg_class goes to the front of rebuildFirstList while
2165+
* pg_class_oid_index goes to the back of rebuildFirstList, so
2166+
* they are done first and second respectively. Other nailed
2167+
* relations go to the front of rebuildList, so they'll be done
2168+
* next in no particular order; and everything else goes to the
2169+
* back of rebuildList.
21522170
*/
2153-
if (relation->rd_isnailed &&
2154-
relation->rd_rel->relkind == RELKIND_INDEX)
2155-
{
2156-
if (RelationGetRelid(relation) == ClassOidIndexId)
2157-
rebuildFirstList = lcons(relation, rebuildFirstList);
2158-
else
2159-
rebuildFirstList = lappend(rebuildFirstList, relation);
2160-
}
2161-
else
2171+
if (RelationGetRelid(relation) == RelationRelationId)
2172+
rebuildFirstList = lcons(relation, rebuildFirstList);
2173+
else if (RelationGetRelid(relation) == ClassOidIndexId)
2174+
rebuildFirstList = lappend(rebuildFirstList, relation);
2175+
else if (relation->rd_isnailed)
21622176
rebuildList = lcons(relation, rebuildList);
2177+
else
2178+
rebuildList = lappend(rebuildList, relation);
21632179
}
21642180
}
21652181

@@ -2170,11 +2186,6 @@ RelationCacheInvalidate(void)
21702186
*/
21712187
smgrcloseall();
21722188

2173-
/*
2174-
* Reload relation mapping data before starting to reconstruct cache.
2175-
*/
2176-
RelationMapInvalidateAll();
2177-
21782189
/* Phase 2: rebuild the items found to need rebuild in phase 1 */
21792190
foreach(l, rebuildFirstList)
21802191
{

0 commit comments

Comments
 (0)