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

Commitab3148b

Browse files
committed
Fix bug in temporary file management with subtransactions. A cursor opened
in a subtransaction stays open even if the subtransaction is aborted, soany temporary files related to it must stay alive as well. With the patch,we use ResourceOwners to track open temporary files and don't automaticallyclose them at subtransaction end (though in the normal case temporary filesare registered with the subtransaction resource owner and will therefore beclosed).At end of top transaction, we still check that there's no temporary filesmarked as close-at-end-of-transaction open, but that's now just a debuggingcross-check as the resource owner cleanup should've closed them already.
1 parentdc58805 commitab3148b

File tree

3 files changed

+146
-34
lines changed

3 files changed

+146
-34
lines changed

‎src/backend/storage/file/fd.c

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.150 2009/08/05 18:01:54 heikki Exp $
10+
* $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.151 2009/12/03 11:03:28 heikki Exp $
1111
*
1212
* NOTES:
1313
*
@@ -55,6 +55,7 @@
5555
#include"storage/fd.h"
5656
#include"storage/ipc.h"
5757
#include"utils/guc.h"
58+
#include"utils/resowner.h"
5859

5960

6061
/*
@@ -134,7 +135,7 @@ typedef struct vfd
134135
{
135136
intfd;/* current FD, or VFD_CLOSED if none */
136137
unsigned shortfdstate;/* bitflags for VFD's state */
137-
SubTransactionIdcreate_subid;/*for TEMPORARY fds, creating subxact */
138+
ResourceOwnerresowner;/*owner, for automatic cleanup */
138139
FilenextFree;/* link to next free VFD, if in freelist */
139140
FilelruMoreRecently;/* doubly linked recency-of-use list */
140141
FilelruLessRecently;
@@ -865,6 +866,7 @@ PathNameOpenFile(FileName fileName, int fileFlags, int fileMode)
865866
vfdP->fileMode=fileMode;
866867
vfdP->seekPos=0;
867868
vfdP->fdstate=0x0;
869+
vfdP->resowner=NULL;
868870

869871
returnfile;
870872
}
@@ -876,11 +878,12 @@ PathNameOpenFile(FileName fileName, int fileFlags, int fileMode)
876878
* There's no need to pass in fileFlags or fileMode either, since only
877879
* one setting makes any sense for a temp file.
878880
*
879-
* interXact: if true, don't close the file at end-of-transaction. In
880-
* most cases, you don't want temporary files to outlive the transaction
881-
* that created them, so this should be false -- but if you need
882-
* "somewhat" temporary storage, this might be useful. In either case,
883-
* the file is removed when the File is explicitly closed.
881+
* Unless interXact is true, the file is remembered by CurrentResourceOwner
882+
* to ensure it's closed and deleted when it's no longer needed, typically at
883+
* the end-of-transaction. In most cases, you don't want temporary files to
884+
* outlive the transaction that created them, so this should be false -- but
885+
* if you need "somewhat" temporary storage, this might be useful. In either
886+
* case, the file is removed when the File is explicitly closed.
884887
*/
885888
File
886889
OpenTemporaryFile(boolinterXact)
@@ -918,11 +921,14 @@ OpenTemporaryFile(bool interXact)
918921
/* Mark it for deletion at close */
919922
VfdCache[file].fdstate |=FD_TEMPORARY;
920923

921-
/*Mark itfor deletion at EOXact */
924+
/*Register itwith the current resource owner */
922925
if (!interXact)
923926
{
924927
VfdCache[file].fdstate |=FD_XACT_TEMPORARY;
925-
VfdCache[file].create_subid=GetCurrentSubTransactionId();
928+
929+
ResourceOwnerEnlargeFiles(CurrentResourceOwner);
930+
ResourceOwnerRememberFile(CurrentResourceOwner,file);
931+
VfdCache[file].resowner=CurrentResourceOwner;
926932

927933
/* ensure cleanup happens at eoxact */
928934
have_xact_temporary_files= true;
@@ -1051,6 +1057,10 @@ FileClose(File file)
10511057
elog(LOG,"could not unlink file \"%s\": %m",vfdP->fileName);
10521058
}
10531059

1060+
/* Unregister it from the resource owner */
1061+
if (vfdP->resowner)
1062+
ResourceOwnerForgetFile(vfdP->resowner,file);
1063+
10541064
/*
10551065
* Return the Vfd slot to the free list
10561066
*/
@@ -1695,24 +1705,6 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid,
16951705
{
16961706
Indexi;
16971707

1698-
if (have_xact_temporary_files)
1699-
{
1700-
Assert(FileIsNotOpen(0));/* Make sure ring not corrupted */
1701-
for (i=1;i<SizeVfdCache;i++)
1702-
{
1703-
unsigned shortfdstate=VfdCache[i].fdstate;
1704-
1705-
if ((fdstate&FD_XACT_TEMPORARY)&&
1706-
VfdCache[i].create_subid==mySubid)
1707-
{
1708-
if (isCommit)
1709-
VfdCache[i].create_subid=parentSubid;
1710-
elseif (VfdCache[i].fileName!=NULL)
1711-
FileClose(i);
1712-
}
1713-
}
1714-
}
1715-
17161708
for (i=0;i<numAllocatedDescs;i++)
17171709
{
17181710
if (allocatedDescs[i].create_subid==mySubid)
@@ -1733,9 +1725,10 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid,
17331725
*
17341726
* This routine is called during transaction commit or abort (it doesn't
17351727
* particularly care which). All still-open per-transaction temporary file
1736-
* VFDs are closed, which also causes the underlying files to be
1737-
* deleted. Furthermore, all "allocated" stdio files are closed.
1738-
* We also forget any transaction-local temp tablespace list.
1728+
* VFDs are closed, which also causes the underlying files to be deleted
1729+
* (although they should've been closed already by the ResourceOwner
1730+
* cleanup). Furthermore, all "allocated" stdio files are closed. We also
1731+
* forget any transaction-local temp tablespace list.
17391732
*/
17401733
void
17411734
AtEOXact_Files(void)
@@ -1787,16 +1780,26 @@ CleanupTempFiles(bool isProcExit)
17871780
/*
17881781
* If we're in the process of exiting a backend process, close
17891782
* all temporary files. Otherwise, only close temporary files
1790-
* local to the current transaction.
1783+
* local to the current transaction. They should be closed
1784+
* by the ResourceOwner mechanism already, so this is just
1785+
* a debugging cross-check.
17911786
*/
1792-
if (isProcExit|| (fdstate&FD_XACT_TEMPORARY))
1787+
if (isProcExit)
1788+
FileClose(i);
1789+
elseif (fdstate&FD_XACT_TEMPORARY)
1790+
{
1791+
elog(WARNING,
1792+
"temporary file %s not closed at end-of-transaction",
1793+
VfdCache[i].fileName);
17931794
FileClose(i);
1795+
}
17941796
}
17951797
}
17961798

17971799
have_xact_temporary_files= false;
17981800
}
17991801

1802+
/* Clean up "allocated" stdio files and dirs. */
18001803
while (numAllocatedDescs>0)
18011804
FreeDesc(&allocatedDescs[0]);
18021805
}

‎src/backend/utils/resowner/resowner.c

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.32 2009/06/11 14:49:06 momjian Exp $
17+
* $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.33 2009/12/03 11:03:29 heikki Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -72,6 +72,11 @@ typedef struct ResourceOwnerData
7272
intnsnapshots;/* number of owned snapshot references */
7373
Snapshot*snapshots;/* dynamically allocated array */
7474
intmaxsnapshots;/* currently allocated array size */
75+
76+
/* We have built-in support for remembering open temporary files */
77+
intnfiles;/* number of owned temporary files */
78+
File*files;/* dynamically allocated array */
79+
intmaxfiles;/* currently allocated array size */
7580
}ResourceOwnerData;
7681

7782

@@ -105,6 +110,7 @@ static void PrintRelCacheLeakWarning(Relation rel);
105110
staticvoidPrintPlanCacheLeakWarning(CachedPlan*plan);
106111
staticvoidPrintTupleDescLeakWarning(TupleDesctupdesc);
107112
staticvoidPrintSnapshotLeakWarning(Snapshotsnapshot);
113+
staticvoidPrintFileLeakWarning(Filefile);
108114

109115

110116
/*****************************************************************************
@@ -316,6 +322,14 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
316322
UnregisterSnapshot(owner->snapshots[owner->nsnapshots-1]);
317323
}
318324

325+
/* Ditto for temporary files */
326+
while (owner->nfiles>0)
327+
{
328+
if (isCommit)
329+
PrintFileLeakWarning(owner->files[owner->nfiles-1]);
330+
FileClose(owner->files[owner->nfiles-1]);
331+
}
332+
319333
/* Clean up index scans too */
320334
ReleaseResources_hash();
321335
}
@@ -347,6 +361,7 @@ ResourceOwnerDelete(ResourceOwner owner)
347361
Assert(owner->nplanrefs==0);
348362
Assert(owner->ntupdescs==0);
349363
Assert(owner->nsnapshots==0);
364+
Assert(owner->nfiles==0);
350365

351366
/*
352367
* Delete children. The recursive call will delink the child from me, so
@@ -377,6 +392,8 @@ ResourceOwnerDelete(ResourceOwner owner)
377392
pfree(owner->tupdescs);
378393
if (owner->snapshots)
379394
pfree(owner->snapshots);
395+
if (owner->files)
396+
pfree(owner->files);
380397

381398
pfree(owner);
382399
}
@@ -1035,3 +1052,87 @@ PrintSnapshotLeakWarning(Snapshot snapshot)
10351052
"Snapshot reference leak: Snapshot %p still referenced",
10361053
snapshot);
10371054
}
1055+
1056+
1057+
/*
1058+
* Make sure there is room for at least one more entry in a ResourceOwner's
1059+
* files reference array.
1060+
*
1061+
* This is separate from actually inserting an entry because if we run out
1062+
* of memory, it's critical to do so *before* acquiring the resource.
1063+
*/
1064+
void
1065+
ResourceOwnerEnlargeFiles(ResourceOwnerowner)
1066+
{
1067+
intnewmax;
1068+
1069+
if (owner->nfiles<owner->maxfiles)
1070+
return;/* nothing to do */
1071+
1072+
if (owner->files==NULL)
1073+
{
1074+
newmax=16;
1075+
owner->files= (File*)
1076+
MemoryContextAlloc(TopMemoryContext,newmax*sizeof(File));
1077+
owner->maxfiles=newmax;
1078+
}
1079+
else
1080+
{
1081+
newmax=owner->maxfiles*2;
1082+
owner->files= (File*)
1083+
repalloc(owner->files,newmax*sizeof(File));
1084+
owner->maxfiles=newmax;
1085+
}
1086+
}
1087+
1088+
/*
1089+
* Remember that a temporary file is owned by a ResourceOwner
1090+
*
1091+
* Caller must have previously done ResourceOwnerEnlargeFiles()
1092+
*/
1093+
void
1094+
ResourceOwnerRememberFile(ResourceOwnerowner,Filefile)
1095+
{
1096+
Assert(owner->nfiles<owner->maxfiles);
1097+
owner->files[owner->nfiles]=file;
1098+
owner->nfiles++;
1099+
}
1100+
1101+
/*
1102+
* Forget that a temporary file is owned by a ResourceOwner
1103+
*/
1104+
void
1105+
ResourceOwnerForgetFile(ResourceOwnerowner,Filefile)
1106+
{
1107+
File*files=owner->files;
1108+
intns1=owner->nfiles-1;
1109+
inti;
1110+
1111+
for (i=ns1;i >=0;i--)
1112+
{
1113+
if (files[i]==file)
1114+
{
1115+
while (i<ns1)
1116+
{
1117+
files[i]=files[i+1];
1118+
i++;
1119+
}
1120+
owner->nfiles=ns1;
1121+
return;
1122+
}
1123+
}
1124+
elog(ERROR,"temporery file %d is not owned by resource owner %s",
1125+
file,owner->name);
1126+
}
1127+
1128+
1129+
/*
1130+
* Debugging subroutine
1131+
*/
1132+
staticvoid
1133+
PrintFileLeakWarning(Filefile)
1134+
{
1135+
elog(WARNING,
1136+
"temporary file leak: File %d still referenced",
1137+
file);
1138+
}

‎src/include/utils/resowner.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@
1212
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
1313
* Portions Copyright (c) 1994, Regents of the University of California
1414
*
15-
* $PostgreSQL: pgsql/src/include/utils/resowner.h,v 1.17 2009/01/01 17:24:02 momjian Exp $
15+
* $PostgreSQL: pgsql/src/include/utils/resowner.h,v 1.18 2009/12/03 11:03:29 heikki Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
1919
#ifndefRESOWNER_H
2020
#defineRESOWNER_H
2121

2222
#include"storage/buf.h"
23+
#include"storage/fd.h"
2324
#include"utils/catcache.h"
2425
#include"utils/plancache.h"
2526
#include"utils/snapshot.h"
@@ -129,4 +130,11 @@ extern void ResourceOwnerRememberSnapshot(ResourceOwner owner,
129130
externvoidResourceOwnerForgetSnapshot(ResourceOwnerowner,
130131
Snapshotsnapshot);
131132

133+
/* support for temporary file management */
134+
externvoidResourceOwnerEnlargeFiles(ResourceOwnerowner);
135+
externvoidResourceOwnerRememberFile(ResourceOwnerowner,
136+
Filefile);
137+
externvoidResourceOwnerForgetFile(ResourceOwnerowner,
138+
Filefile);
139+
132140
#endif/* RESOWNER_H */

0 commit comments

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