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

Commit 76ccf73

Browse files
committed
Repair problem exposed by Jan's new parallel-regression-test scaffold:
inval.c thought it could safely use the catcache to look up the OIDs of system relations. Not good, considering that inval.c could be called during catcache loading, if a shared-inval message arrives. Rip out the lookup logic and instead use the known OIDs from pg_class.h.
1 parent 9ba0172 commit 76ccf73

File tree

6 files changed

+61
-149
lines changed

6 files changed

+61
-149
lines changed

src/backend/utils/cache/catcache.c

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v 1.52 1999/11/19 18:51:48 wieck Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v 1.53 1999/11/21 01:58:22 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -819,7 +819,7 @@ InitSysCache(char *relname,
819819
* --------------------------------
820820
*/
821821
static HeapTuple
822-
SearchSelfReferences(const struct catcache * cache)
822+
SearchSelfReferences(struct catcache * cache)
823823
{
824824
HeapTuple ntp;
825825
Relation rel;
@@ -983,23 +983,11 @@ SearchSysCache(struct catcache * cache,
983983
* ----------------
984984
*/
985985

986-
/* ----------
987-
* It is definitely insufficient. While modifying the regression
988-
* test to run independent tests concurrently it happened, that
989-
* this code fails VERY often. ISTM that 'cache' points into
990-
* shared memory, but that 'busy' means this backend is loading
991-
* a new entry. So when another backend has set busy, this one
992-
* think's it detected a recursion.
993-
*
994-
* Need's a smarter detection mechanism - Jan
995-
*
996986
if (cache->busy)
997987
{
998988
elog(ERROR, "SearchSysCache: recursive use of cache %d", cache->id);
999989
}
1000990
cache->busy = true;
1001-
* ----------
1002-
*/
1003991

1004992
/* ----------------
1005993
* open the relation associated with the cache

src/backend/utils/cache/inval.c

Lines changed: 47 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/utils/cache/inval.c,v 1.29 1999/11/17 23:51:21 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/utils/cache/inval.c,v 1.30 1999/11/21 01:58:22 tgl Exp $
1111
*
1212
* Note - this code is real crufty...
1313
*
@@ -18,23 +18,36 @@
1818
#include "catalog/catalog.h"
1919
#include "catalog/catname.h"
2020
#include "catalog/heap.h"
21+
#include "catalog/pg_class.h"
2122
#include "miscadmin.h"
2223
#include "storage/sinval.h"
2324
#include "utils/catcache.h"
2425
#include "utils/inval.h"
2526
#include "utils/relcache.h"
2627

27-
static InvalidationEntry InvalidationEntryAllocate(uint16 size);
28-
static void LocalInvalidInvalidate(LocalInvalid invalid, void (*function) ());
29-
static LocalInvalid LocalInvalidRegister(LocalInvalid invalid,
30-
InvalidationEntry entry);
31-
static void getmyrelids(void);
32-
3328

3429
/* ----------------
3530
* private invalidation structures
3631
* ----------------
3732
*/
33+
34+
typedef struct InvalidationUserData
35+
{
36+
struct InvalidationUserData *dataP[1]; /* VARIABLE LENGTH */
37+
} InvalidationUserData; /* VARIABLE LENGTH STRUCTURE */
38+
39+
typedef struct InvalidationEntryData
40+
{
41+
InvalidationUserData *nextP;
42+
InvalidationUserData userData; /* VARIABLE LENGTH ARRAY */
43+
} InvalidationEntryData; /* VARIABLE LENGTH STRUCTURE */
44+
45+
typedef Pointer InvalidationEntry;
46+
47+
typedef InvalidationEntry LocalInvalid;
48+
49+
#define EmptyLocalInvalid NULL
50+
3851
typedef struct CatalogInvalidationData
3952
{
4053
Index cacheId;
@@ -66,15 +79,14 @@ typedef InvalidationMessageData *InvalidationMessage;
6679
* variables and macros
6780
* ----------------
6881
*/
69-
static LocalInvalid Invalid = EmptyLocalInvalid; /* XXX global */
82+
static LocalInvalid Invalid = EmptyLocalInvalid; /* head of linked list */
7083

71-
Oid MyRelationRelationId = InvalidOid;
72-
Oid MyAttributeRelationId = InvalidOid;
73-
Oid MyAMRelationId = InvalidOid;
74-
Oid MyAMOPRelationId = InvalidOid;
7584

76-
#define ValidateHacks() \
77-
if (!OidIsValid(MyRelationRelationId)) getmyrelids()
85+
static InvalidationEntry InvalidationEntryAllocate(uint16 size);
86+
static void LocalInvalidInvalidate(LocalInvalid invalid, void (*function) ());
87+
static LocalInvalid LocalInvalidRegister(LocalInvalid invalid,
88+
InvalidationEntry entry);
89+
7890

7991
/* ----------------------------------------------------------------
8092
* "local" invalidation support functions
@@ -99,7 +111,8 @@ InvalidationEntryAllocate(uint16 size)
99111

100112
/* --------------------------------
101113
* LocalInvalidRegister
102-
* Returns a new local cache invalidation state containing a new entry.
114+
* Link an invalidation entry into a chain of them. Really ugly
115+
* coding here.
103116
* --------------------------------
104117
*/
105118
static LocalInvalid
@@ -117,7 +130,7 @@ LocalInvalidRegister(LocalInvalid invalid,
117130
/* --------------------------------
118131
* LocalInvalidInvalidate
119132
* Processes, then frees all entries in a local cache
120-
* invalidation state.
133+
* invalidation list.
121134
* --------------------------------
122135
*/
123136
static void
@@ -187,7 +200,7 @@ CacheIdRegisterLocalInvalid(Index cacheId,
187200
ItemPointerCopy(pointer, &message->any.catalog.pointerData);
188201

189202
/* ----------------
190-
* Note: Invalid is a global variable
203+
* Add message to linked list of unprocessed messages.
191204
* ----------------
192205
*/
193206
Invalid = LocalInvalidRegister(Invalid, (InvalidationEntry) message);
@@ -224,32 +237,12 @@ RelationIdRegisterLocalInvalid(Oid relationId, Oid objectId)
224237
message->any.relation.objectId = objectId;
225238

226239
/* ----------------
227-
* Note: Invalid is a global variable
240+
* Add message to linked list of unprocessed messages.
228241
* ----------------
229242
*/
230243
Invalid = LocalInvalidRegister(Invalid, (InvalidationEntry) message);
231244
}
232245

233-
/* --------------------------------
234-
* getmyrelids
235-
* --------------------------------
236-
*/
237-
static void
238-
getmyrelids()
239-
{
240-
MyRelationRelationId = RelnameFindRelid(RelationRelationName);
241-
Assert(RelationRelationName != InvalidOid);
242-
243-
MyAttributeRelationId = RelnameFindRelid(AttributeRelationName);
244-
Assert(AttributeRelationName != InvalidOid);
245-
246-
MyAMRelationId = RelnameFindRelid(AccessMethodRelationName);
247-
Assert(MyAMRelationId != InvalidOid);
248-
249-
MyAMOPRelationId = RelnameFindRelid(AccessMethodOperatorRelationName);
250-
Assert(MyAMOPRelationId != InvalidOid);
251-
}
252-
253246
/* --------------------------------
254247
* CacheIdInvalidate
255248
*
@@ -284,38 +277,23 @@ CacheIdInvalidate(Index cacheId,
284277

285278
CacheIdInvalidate_DEBUG1;
286279

287-
ValidateHacks(); /* XXX */
288-
289280
/* ----------------
290-
* if the cacheId is the oid of any of the tuples in the
291-
* following system relations, then assume we are invalidating
292-
* a relation descriptor
281+
* if the cacheId is the oid of any of the following system relations,
282+
* then assume we are invalidating a relation descriptor
293283
* ----------------
294284
*/
295-
if (cacheId == MyRelationRelationId)
285+
if (cacheId == RelOid_pg_class)
296286
{
297287
RelationIdInvalidateRelationCacheByRelationId(hashIndex);
298288
return;
299289
}
300290

301-
if (cacheId == MyAttributeRelationId)
291+
if (cacheId == RelOid_pg_attribute)
302292
{
303293
RelationIdInvalidateRelationCacheByRelationId(hashIndex);
304294
return;
305295
}
306296

307-
if (cacheId == MyAMRelationId)
308-
{
309-
RelationIdInvalidateRelationCacheByAccessMethodId(hashIndex);
310-
return;
311-
}
312-
313-
if (cacheId == MyAMOPRelationId)
314-
{
315-
RelationIdInvalidateRelationCacheByAccessMethodId(InvalidOid);
316-
return;
317-
}
318-
319297
/* ----------------
320298
* Yow! the caller asked us to invalidate something else.
321299
* ----------------
@@ -446,29 +424,22 @@ RelationInvalidateRelationCache(Relation relation,
446424
void (*function) ())
447425
{
448426
Oid relationId;
449-
Oid objectId = (Oid) 0;
427+
Oid objectId;
450428

451429
/* ----------------
452430
* get the relation object id
453431
* ----------------
454432
*/
455-
ValidateHacks(); /* XXX */
456433
relationId = RelationGetRelid(relation);
457434

458435
/* ----------------
459-
*
436+
* is it one of the ones we need to send an SI message for?
460437
* ----------------
461438
*/
462-
if (relationId == MyRelationRelationId)
439+
if (relationId == RelOid_pg_class)
463440
objectId = tuple->t_data->t_oid;
464-
else if (relationId == MyAttributeRelationId)
441+
else if (relationId == RelOid_pg_attribute)
465442
objectId = ((Form_pg_attribute) GETSTRUCT(tuple))->attrelid;
466-
else if (relationId == MyAMRelationId)
467-
objectId = tuple->t_data->t_oid;
468-
else if (relationId == MyAMOPRelationId)
469-
{
470-
; /* objectId is unused */
471-
}
472443
else
473444
return;
474445

@@ -482,19 +453,6 @@ RelationInvalidateRelationCache(Relation relation,
482453
}
483454

484455

485-
/*
486-
* InitLocalInvalidateData
487-
*
488-
* Setup this before anything could ever get invalid!
489-
* Called by InitPostgres();
490-
*/
491-
void
492-
InitLocalInvalidateData()
493-
{
494-
ValidateHacks();
495-
}
496-
497-
498456
/*
499457
* DiscardInvalid
500458
* Causes the invalidated cache state to be discarded.
@@ -528,6 +486,8 @@ DiscardInvalid()
528486
void
529487
RegisterInvalid(bool send)
530488
{
489+
LocalInvalid invalid;
490+
531491
/* ----------------
532492
* debugging stuff
533493
* ----------------
@@ -537,17 +497,19 @@ RegisterInvalid(bool send)
537497
#endif /* defined(INVALIDDEBUG) */
538498

539499
/* ----------------
540-
* Note: Invalid is a global variable
500+
* Process and free the current list of inval messages.
541501
* ----------------
542502
*/
503+
invalid = Invalid;
504+
Invalid = EmptyLocalInvalid; /* anything added now is part of a new list */
505+
543506
if (send)
544-
LocalInvalidInvalidate(Invalid,
507+
LocalInvalidInvalidate(invalid,
545508
InvalidationMessageRegisterSharedInvalid);
546509
else
547-
LocalInvalidInvalidate(Invalid,
510+
LocalInvalidInvalidate(invalid,
548511
InvalidationMessageCacheInvalidate);
549512

550-
Invalid = EmptyLocalInvalid;
551513
}
552514

553515
/*

src/backend/utils/cache/relcache.c

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.79 1999/11/18 13:56:28 wieck Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.80 1999/11/21 01:58:22 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -209,11 +209,6 @@ do { \
209209
static void formrdesc(char *relationName, u_int natts,
210210
FormData_pg_attribute *att);
211211

212-
#ifdef NOT_USED /* See comments at line 1304 */
213-
static void RelationFlushIndexes(Relation *r, Oid accessMethodId);
214-
215-
#endif
216-
217212
static HeapTuple ScanPgRelation(RelationBuildDescInfo buildinfo);
218213
static HeapTuple scan_pg_rel_seq(RelationBuildDescInfo buildinfo);
219214
static HeapTuple scan_pg_rel_ind(RelationBuildDescInfo buildinfo);
@@ -1431,12 +1426,9 @@ RelationIdInvalidateRelationCacheByRelationId(Oid relationId)
14311426
}
14321427
}
14331428

1434-
#if NOT_USED /* See comments at line 1304 */
1435-
/* --------------------------------
1436-
* RelationIdInvalidateRelationCacheByAccessMethodId
1437-
*
1438-
* RelationFlushIndexes is needed for use with HashTableWalk..
1439-
* --------------------------------
1429+
#if NOT_USED
1430+
/* only used by RelationIdInvalidateRelationCacheByAccessMethodId,
1431+
* which is dead code.
14401432
*/
14411433
static void
14421434
RelationFlushIndexes(Relation *r,
@@ -1459,26 +1451,26 @@ RelationFlushIndexes(Relation *r,
14591451
#endif
14601452

14611453

1454+
#if NOT_USED
14621455
void
14631456
RelationIdInvalidateRelationCacheByAccessMethodId(Oid accessMethodId)
14641457
{
1465-
#ifdef NOT_USED
1466-
14671458
/*
14681459
* 25 aug 1992: mao commented out the ht walk below. it should be
14691460
* doing the right thing, in theory, but flushing reldescs for index
14701461
* relations apparently doesn't work. we want to cut 4.0.1, and i
14711462
* don't want to introduce new bugs. this code never executed before,
14721463
* so i'm turning it off for now. after the release is cut, i'll fix
14731464
* this up.
1465+
*
1466+
* 20 nov 1999: this code has still never done anything, so I'm
1467+
* cutting the routine out of the system entirely. tgl
14741468
*/
14751469

14761470
HashTableWalk(RelationNameCache, (HashtFunc) RelationFlushIndexes,
14771471
accessMethodId);
1478-
#else
1479-
return;
1480-
#endif
14811472
}
1473+
#endif
14821474

14831475
/*
14841476
* RelationCacheInvalidate

src/backend/utils/init/postinit.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.52 1999/10/25 03:07:51 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.53 1999/11/21 01:58:21 tgl Exp $
1111
*
1212
* NOTES
1313
* InitPostgres() is the function called from PostgresMain
@@ -553,12 +553,6 @@ InitPostgres(char *name) /* database name */
553553
*/
554554
InitUserid();
555555

556-
/*
557-
* Initialize local data in cache invalidation stuff
558-
*/
559-
if (!bootstrap)
560-
InitLocalInvalidateData();
561-
562556
if (lockingOff)
563557
LockDisable(true);
564558

0 commit comments

Comments
 (0)