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

Commit8efa301

Browse files
committed
Be more paranoid about OOM in search_path cache.
Recent commitf26c236 introduced a search_path cache, but left somepotential out-of-memory hazards. Simplify the code and make it saferagainst OOM.This change reintroduces one list_copy(), losing a small amount of theperformance gained inf26c236. A future change may optimize awaythe list_copy() again if it can be done in a safer way.Discussion:https://postgr.es/m/e6fded24cb8a2c53d4ef069d9f69cc7baaafe9ef.camel@j-davis.com
1 parent3650e7a commit8efa301

File tree

1 file changed

+36
-83
lines changed

1 file changed

+36
-83
lines changed

‎src/backend/catalog/namespace.c

Lines changed: 36 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -279,42 +279,23 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
279279
staticnsphash_hash*SearchPathCache=NULL;
280280

281281
/*
282-
* Createsearch pathcache.
282+
* Createor reset search_pathcache as necessary.
283283
*/
284284
staticvoid
285285
spcache_init(void)
286286
{
287287
Assert(SearchPathCacheContext);
288288

289-
if (SearchPathCache)
289+
if (SearchPathCache&&searchPathCacheValid&&
290+
SearchPathCache->members<SPCACHE_RESET_THRESHOLD)
290291
return;
291292

293+
MemoryContextReset(SearchPathCacheContext);
292294
/* arbitrary initial starting size of 16 elements */
293295
SearchPathCache=nsphash_create(SearchPathCacheContext,16,NULL);
294296
searchPathCacheValid= true;
295297
}
296298

297-
/*
298-
* Reset and reinitialize search path cache.
299-
*/
300-
staticvoid
301-
spcache_reset(void)
302-
{
303-
Assert(SearchPathCacheContext);
304-
Assert(SearchPathCache);
305-
306-
MemoryContextReset(SearchPathCacheContext);
307-
SearchPathCache=NULL;
308-
309-
spcache_init();
310-
}
311-
312-
staticuint32
313-
spcache_members(void)
314-
{
315-
returnSearchPathCache->members;
316-
}
317-
318299
/*
319300
* Look up or insert entry in search path cache.
320301
*
@@ -325,27 +306,25 @@ static SearchPathCacheEntry *
325306
spcache_insert(constchar*searchPath,Oidroleid)
326307
{
327308
SearchPathCacheEntry*entry;
328-
boolfound;
329309
SearchPathCacheKeycachekey= {
330310
.searchPath=searchPath,
331311
.roleid=roleid
332312
};
333313

334314
/*
335-
* If a new entry is created, we must ensure that it's properly
336-
* initialized. Set the cache invalid temporarily, so that if the
337-
* MemoryContextStrdup() below raises an OOM, the cache will be reset on
338-
* the next use, clearing the uninitialized entry.
315+
* searchPath is not saved in SearchPathCacheContext. First perform a
316+
* lookup, and copy searchPath only if we need to create a new entry.
339317
*/
340-
searchPathCacheValid= false;
341-
342-
entry=nsphash_insert(SearchPathCache,cachekey,&found);
318+
entry=nsphash_lookup(SearchPathCache,cachekey);
343319

344-
/* ensure that key is initialized and the rest is zeroed */
345-
if (!found)
320+
if (!entry)
346321
{
347-
entry->key.searchPath=MemoryContextStrdup(SearchPathCacheContext,searchPath);
348-
entry->key.roleid=roleid;
322+
boolfound;
323+
324+
cachekey.searchPath=MemoryContextStrdup(SearchPathCacheContext,searchPath);
325+
entry=nsphash_insert(SearchPathCache,cachekey,&found);
326+
Assert(!found);
327+
349328
entry->oidlist=NIL;
350329
entry->finalPath=NIL;
351330
entry->firstNS=InvalidOid;
@@ -354,7 +333,6 @@ spcache_insert(const char *searchPath, Oid roleid)
354333
/* do not touch entry->status, used by simplehash */
355334
}
356335

357-
searchPathCacheValid= true;
358336
returnentry;
359337
}
360338

@@ -4183,31 +4161,15 @@ finalNamespacePath(List *oidlist, Oid *firstNS)
41834161
/*
41844162
* Retrieve search path information from the cache; or if not there, fill
41854163
* it. The returned entry is valid only until the next call to this function.
4186-
*
4187-
* We also determine if the newly-computed finalPath is the same as the
4188-
* prevPath passed by the caller (i.e. a no-op or a real change?). It's more
4189-
* efficient to check for a change in this function than the caller, because
4190-
* we can avoid unnecessary temporary copies of the previous path.
41914164
*/
41924165
staticconstSearchPathCacheEntry*
4193-
cachedNamespacePath(constchar*searchPath,Oidroleid,List*prevPath,
4194-
bool*same)
4166+
cachedNamespacePath(constchar*searchPath,Oidroleid)
41954167
{
41964168
MemoryContextoldcxt;
41974169
SearchPathCacheEntry*entry;
4198-
List*prevPathCopy=NIL;
41994170

42004171
spcache_init();
42014172

4202-
/* invalidate cache if necessary */
4203-
if (!searchPathCacheValid||spcache_members() >=SPCACHE_RESET_THRESHOLD)
4204-
{
4205-
/* prevPath will be destroyed; make temp copy for later comparison */
4206-
prevPathCopy=list_copy(prevPath);
4207-
prevPath=prevPathCopy;
4208-
spcache_reset();
4209-
}
4210-
42114173
entry=spcache_insert(searchPath,roleid);
42124174

42134175
/*
@@ -4232,38 +4194,22 @@ cachedNamespacePath(const char *searchPath, Oid roleid, List *prevPath,
42324194
if (entry->finalPath==NIL||object_access_hook||
42334195
entry->forceRecompute)
42344196
{
4235-
/*
4236-
* Do not free the stale value of entry->finalPath until we've
4237-
* performed the comparison, in case it's aliased by prevPath (which
4238-
* can only happen when recomputing due to an object_access_hook).
4239-
*/
4240-
List*finalPath;
4197+
list_free(entry->finalPath);
4198+
entry->finalPath=NIL;
42414199

42424200
oldcxt=MemoryContextSwitchTo(SearchPathCacheContext);
4243-
finalPath=finalNamespacePath(entry->oidlist,
4244-
&entry->firstNS);
4201+
entry->finalPath=finalNamespacePath(entry->oidlist,
4202+
&entry->firstNS);
42454203
MemoryContextSwitchTo(oldcxt);
42464204

4247-
*same=equal(prevPath,finalPath);
4248-
4249-
list_free(entry->finalPath);
4250-
entry->finalPath=finalPath;
4251-
42524205
/*
4253-
* If an object_access_hook set when finalPath is calculated, the
4206+
* If an object_access_hookisset when finalPath is calculated, the
42544207
* result may be affected by the hook. Force recomputation of
42554208
* finalPath the next time this cache entry is used, even if the
42564209
* object_access_hook is not set at that time.
42574210
*/
42584211
entry->forceRecompute=object_access_hook ? true : false;
42594212
}
4260-
else
4261-
{
4262-
/* use cached version of finalPath */
4263-
*same=equal(prevPath,entry->finalPath);
4264-
}
4265-
4266-
list_free(prevPathCopy);
42674213

42684214
returnentry;
42694215
}
@@ -4275,32 +4221,39 @@ static void
42754221
recomputeNamespacePath(void)
42764222
{
42774223
Oidroleid=GetUserId();
4278-
boolnewPathEqual;
42794224
boolpathChanged;
42804225
constSearchPathCacheEntry*entry;
42814226

42824227
/* Do nothing if path is already valid. */
42834228
if (baseSearchPathValid&&namespaceUser==roleid)
42844229
return;
42854230

4286-
entry=cachedNamespacePath(namespace_search_path,roleid,baseSearchPath,
4287-
&newPathEqual);
4231+
entry=cachedNamespacePath(namespace_search_path,roleid);
42884232

42894233
if (baseCreationNamespace==entry->firstNS&&
42904234
baseTempCreationPending==entry->temp_missing&&
4291-
newPathEqual)
4235+
equal(entry->finalPath,baseSearchPath))
42924236
{
42934237
pathChanged= false;
42944238
}
42954239
else
42964240
{
4241+
MemoryContextoldcxt;
4242+
List*newpath;
4243+
42974244
pathChanged= true;
4298-
}
42994245

4300-
/* Now safe to assign to state variables. */
4301-
baseSearchPath=entry->finalPath;
4302-
baseCreationNamespace=entry->firstNS;
4303-
baseTempCreationPending=entry->temp_missing;
4246+
/* Must save OID list in permanent storage. */
4247+
oldcxt=MemoryContextSwitchTo(TopMemoryContext);
4248+
newpath=list_copy(entry->finalPath);
4249+
MemoryContextSwitchTo(oldcxt);
4250+
4251+
/* Now safe to assign to state variables. */
4252+
list_free(baseSearchPath);
4253+
baseSearchPath=newpath;
4254+
baseCreationNamespace=entry->firstNS;
4255+
baseTempCreationPending=entry->temp_missing;
4256+
}
43044257

43054258
/* Mark the path valid. */
43064259
baseSearchPathValid= true;

0 commit comments

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