Commit 9bacf0e3 authored by Tom Lane's avatar Tom Lane

Revert "Use "transient" files for blind writes, take 2".

This reverts commit fba105b1.
That approach had problems with the smgr-level state not tracking what
we really want to happen, and with the VFD-level state not tracking the
smgr-level state very well either.  In consequence, it was still possible
to hold kernel file descriptors open for long-gone tables (as in recent
report from Tore Halset), and yet there were also cases of FDs being closed
undesirably soon.  A replacement implementation will follow.
parent a66ee69a
...@@ -1882,10 +1882,7 @@ BufferGetTag(Buffer buffer, RelFileNode *rnode, ForkNumber *forknum, ...@@ -1882,10 +1882,7 @@ BufferGetTag(Buffer buffer, RelFileNode *rnode, ForkNumber *forknum,
* written.) * written.)
* *
* If the caller has an smgr reference for the buffer's relation, pass it * If the caller has an smgr reference for the buffer's relation, pass it
* as the second parameter. If not, pass NULL. In the latter case, the * as the second parameter. If not, pass NULL.
* relation will be marked as "transient" so that the corresponding
* kernel-level file descriptors are closed when the current transaction ends,
* if any.
*/ */
static void static void
FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln) FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln)
...@@ -1909,12 +1906,9 @@ FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln) ...@@ -1909,12 +1906,9 @@ FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln)
errcontext.previous = error_context_stack; errcontext.previous = error_context_stack;
error_context_stack = &errcontext; error_context_stack = &errcontext;
/* Find smgr relation for buffer, and mark it as transient */ /* Find smgr relation for buffer */
if (reln == NULL) if (reln == NULL)
{
reln = smgropen(buf->tag.rnode, InvalidBackendId); reln = smgropen(buf->tag.rnode, InvalidBackendId);
smgrsettransient(reln);
}
TRACE_POSTGRESQL_BUFFER_FLUSH_START(buf->tag.forkNum, TRACE_POSTGRESQL_BUFFER_FLUSH_START(buf->tag.forkNum,
buf->tag.blockNum, buf->tag.blockNum,
......
...@@ -126,8 +126,6 @@ int max_safe_fds = 32; /* default if not changed */ ...@@ -126,8 +126,6 @@ int max_safe_fds = 32; /* default if not changed */
/* these are the assigned bits in fdstate below: */ /* these are the assigned bits in fdstate below: */
#define FD_TEMPORARY (1 << 0) /* T = delete when closed */ #define FD_TEMPORARY (1 << 0) /* T = delete when closed */
#define FD_XACT_TEMPORARY (1 << 1) /* T = delete at eoXact */ #define FD_XACT_TEMPORARY (1 << 1) /* T = delete at eoXact */
#define FD_XACT_TRANSIENT (1 << 2) /* T = close (not delete) at aoXact,
* but keep VFD */
typedef struct vfd typedef struct vfd
{ {
...@@ -158,8 +156,11 @@ static Size SizeVfdCache = 0; ...@@ -158,8 +156,11 @@ static Size SizeVfdCache = 0;
*/ */
static int nfile = 0; static int nfile = 0;
/* True if there are files to close/delete at end of transaction */ /*
static bool have_pending_fd_cleanup = false; * Flag to tell whether it's worth scanning VfdCache looking for temp files
* to close
*/
static bool have_xact_temporary_files = false;
/* /*
* Tracks the total size of all temporary files. Note: when temp_file_limit * Tracks the total size of all temporary files. Note: when temp_file_limit
...@@ -607,7 +608,6 @@ LruDelete(File file) ...@@ -607,7 +608,6 @@ LruDelete(File file)
Vfd *vfdP; Vfd *vfdP;
Assert(file != 0); Assert(file != 0);
Assert(!FileIsNotOpen(file));
DO_DB(elog(LOG, "LruDelete %d (%s)", DO_DB(elog(LOG, "LruDelete %d (%s)",
file, VfdCache[file].fileName)); file, VfdCache[file].fileName));
...@@ -971,7 +971,7 @@ OpenTemporaryFile(bool interXact) ...@@ -971,7 +971,7 @@ OpenTemporaryFile(bool interXact)
VfdCache[file].resowner = CurrentResourceOwner; VfdCache[file].resowner = CurrentResourceOwner;
/* ensure cleanup happens at eoxact */ /* ensure cleanup happens at eoxact */
have_pending_fd_cleanup = true; have_xact_temporary_files = true;
} }
return file; return file;
...@@ -1044,25 +1044,6 @@ OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError) ...@@ -1044,25 +1044,6 @@ OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
return file; return file;
} }
/*
* Set the transient flag on a file
*
* This flag tells CleanupTempFiles to close the kernel-level file descriptor
* (but not the VFD itself) at end of transaction.
*/
void
FileSetTransient(File file)
{
Vfd *vfdP;
Assert(FileIsValid(file));
vfdP = &VfdCache[file];
vfdP->fdstate |= FD_XACT_TRANSIENT;
have_pending_fd_cleanup = true;
}
/* /*
* close a file when done with it * close a file when done with it
*/ */
...@@ -1863,9 +1844,8 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid, ...@@ -1863,9 +1844,8 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid,
* particularly care which). All still-open per-transaction temporary file * particularly care which). All still-open per-transaction temporary file
* VFDs are closed, which also causes the underlying files to be deleted * VFDs are closed, which also causes the underlying files to be deleted
* (although they should've been closed already by the ResourceOwner * (although they should've been closed already by the ResourceOwner
* cleanup). Transient files have their kernel file descriptors closed. * cleanup). Furthermore, all "allocated" stdio files are closed. We also
* Furthermore, all "allocated" stdio files are closed. We also forget any * forget any transaction-local temp tablespace list.
* transaction-local temp tablespace list.
*/ */
void void
AtEOXact_Files(void) AtEOXact_Files(void)
...@@ -1888,10 +1868,7 @@ AtProcExit_Files(int code, Datum arg) ...@@ -1888,10 +1868,7 @@ AtProcExit_Files(int code, Datum arg)
} }
/* /*
* General cleanup routine for fd.c. * Close temporary files and delete their underlying files.
*
* Temporary files are closed, and their underlying files deleted.
* Transient files are closed.
* *
* isProcExit: if true, this is being called as the backend process is * isProcExit: if true, this is being called as the backend process is
* exiting. If that's the case, we should remove all temporary files; if * exiting. If that's the case, we should remove all temporary files; if
...@@ -1908,51 +1885,35 @@ CleanupTempFiles(bool isProcExit) ...@@ -1908,51 +1885,35 @@ CleanupTempFiles(bool isProcExit)
* Careful here: at proc_exit we need extra cleanup, not just * Careful here: at proc_exit we need extra cleanup, not just
* xact_temporary files. * xact_temporary files.
*/ */
if (isProcExit || have_pending_fd_cleanup) if (isProcExit || have_xact_temporary_files)
{ {
Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */
for (i = 1; i < SizeVfdCache; i++) for (i = 1; i < SizeVfdCache; i++)
{ {
unsigned short fdstate = VfdCache[i].fdstate; unsigned short fdstate = VfdCache[i].fdstate;
if (VfdCache[i].fileName != NULL) if ((fdstate & FD_TEMPORARY) && VfdCache[i].fileName != NULL)
{ {
if (fdstate & FD_TEMPORARY) /*
{ * If we're in the process of exiting a backend process, close
/* * all temporary files. Otherwise, only close temporary files
* If we're in the process of exiting a backend process, * local to the current transaction. They should be closed by
* close all temporary files. Otherwise, only close * the ResourceOwner mechanism already, so this is just a
* temporary files local to the current transaction. They * debugging cross-check.
* should be closed by the ResourceOwner mechanism */
* already, so this is just a debugging cross-check. if (isProcExit)
*/ FileClose(i);
if (isProcExit) else if (fdstate & FD_XACT_TEMPORARY)
FileClose(i);
else if (fdstate & FD_XACT_TEMPORARY)
{
elog(WARNING,
"temporary file %s not closed at end-of-transaction",
VfdCache[i].fileName);
FileClose(i);
}
}
else if (fdstate & FD_XACT_TRANSIENT)
{ {
/* elog(WARNING,
* Close the FD, and remove the entry from the LRU ring, "temporary file %s not closed at end-of-transaction",
* but also remove the flag from the VFD. This is to VfdCache[i].fileName);
* ensure that if the VFD is reused in the future for FileClose(i);
* non-transient access, we don't close it inappropriately
* then.
*/
if (!FileIsNotOpen(i))
LruDelete(i);
VfdCache[i].fdstate &= ~FD_XACT_TRANSIENT;
} }
} }
} }
have_pending_fd_cleanup = false; have_xact_temporary_files = false;
} }
/* Clean up "allocated" stdio files and dirs. */ /* Clean up "allocated" stdio files and dirs. */
......
...@@ -300,9 +300,6 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo) ...@@ -300,9 +300,6 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
pfree(path); pfree(path);
if (reln->smgr_transient)
FileSetTransient(fd);
reln->md_fd[forkNum] = _fdvec_alloc(); reln->md_fd[forkNum] = _fdvec_alloc();
reln->md_fd[forkNum]->mdfd_vfd = fd; reln->md_fd[forkNum]->mdfd_vfd = fd;
...@@ -585,9 +582,6 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior) ...@@ -585,9 +582,6 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
pfree(path); pfree(path);
if (reln->smgr_transient)
FileSetTransient(fd);
reln->md_fd[forknum] = mdfd = _fdvec_alloc(); reln->md_fd[forknum] = mdfd = _fdvec_alloc();
mdfd->mdfd_vfd = fd; mdfd->mdfd_vfd = fd;
...@@ -1680,9 +1674,6 @@ _mdfd_openseg(SMgrRelation reln, ForkNumber forknum, BlockNumber segno, ...@@ -1680,9 +1674,6 @@ _mdfd_openseg(SMgrRelation reln, ForkNumber forknum, BlockNumber segno,
if (fd < 0) if (fd < 0)
return NULL; return NULL;
if (reln->smgr_transient)
FileSetTransient(fd);
/* allocate an mdfdvec entry for it */ /* allocate an mdfdvec entry for it */
v = _fdvec_alloc(); v = _fdvec_alloc();
......
...@@ -163,33 +163,16 @@ smgropen(RelFileNode rnode, BackendId backend) ...@@ -163,33 +163,16 @@ smgropen(RelFileNode rnode, BackendId backend)
reln->smgr_targblock = InvalidBlockNumber; reln->smgr_targblock = InvalidBlockNumber;
reln->smgr_fsm_nblocks = InvalidBlockNumber; reln->smgr_fsm_nblocks = InvalidBlockNumber;
reln->smgr_vm_nblocks = InvalidBlockNumber; reln->smgr_vm_nblocks = InvalidBlockNumber;
reln->smgr_transient = false;
reln->smgr_which = 0; /* we only have md.c at present */ reln->smgr_which = 0; /* we only have md.c at present */
/* mark it not open */ /* mark it not open */
for (forknum = 0; forknum <= MAX_FORKNUM; forknum++) for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
reln->md_fd[forknum] = NULL; reln->md_fd[forknum] = NULL;
} }
else
/* if it was transient before, it no longer is */
reln->smgr_transient = false;
return reln; return reln;
} }
/*
* smgrsettransient() -- mark an SMgrRelation object as transaction-bound
*
* The main effect of this is that all opened files are marked to be
* kernel-level closed (but not necessarily VFD-closed) when the current
* transaction ends.
*/
void
smgrsettransient(SMgrRelation reln)
{
reln->smgr_transient = true;
}
/* /*
* smgrsetowner() -- Establish a long-lived reference to an SMgrRelation object * smgrsetowner() -- Establish a long-lived reference to an SMgrRelation object
* *
......
...@@ -66,7 +66,6 @@ extern int max_safe_fds; ...@@ -66,7 +66,6 @@ extern int max_safe_fds;
/* Operations on virtual Files --- equivalent to Unix kernel file ops */ /* Operations on virtual Files --- equivalent to Unix kernel file ops */
extern File PathNameOpenFile(FileName fileName, int fileFlags, int fileMode); extern File PathNameOpenFile(FileName fileName, int fileFlags, int fileMode);
extern File OpenTemporaryFile(bool interXact); extern File OpenTemporaryFile(bool interXact);
extern void FileSetTransient(File file);
extern void FileClose(File file); extern void FileClose(File file);
extern int FilePrefetch(File file, off_t offset, int amount); extern int FilePrefetch(File file, off_t offset, int amount);
extern int FileRead(File file, char *buffer, int amount); extern int FileRead(File file, char *buffer, int amount);
......
...@@ -60,7 +60,6 @@ typedef struct SMgrRelationData ...@@ -60,7 +60,6 @@ typedef struct SMgrRelationData
* submodules. Do not touch them from elsewhere. * submodules. Do not touch them from elsewhere.
*/ */
int smgr_which; /* storage manager selector */ int smgr_which; /* storage manager selector */
bool smgr_transient; /* T if files are to be closed at EOXact */
/* for md.c; NULL for forks that are not open */ /* for md.c; NULL for forks that are not open */
struct _MdfdVec *md_fd[MAX_FORKNUM + 1]; struct _MdfdVec *md_fd[MAX_FORKNUM + 1];
...@@ -73,7 +72,6 @@ typedef SMgrRelationData *SMgrRelation; ...@@ -73,7 +72,6 @@ typedef SMgrRelationData *SMgrRelation;
extern void smgrinit(void); extern void smgrinit(void);
extern SMgrRelation smgropen(RelFileNode rnode, BackendId backend); extern SMgrRelation smgropen(RelFileNode rnode, BackendId backend);
extern void smgrsettransient(SMgrRelation reln);
extern bool smgrexists(SMgrRelation reln, ForkNumber forknum); extern bool smgrexists(SMgrRelation reln, ForkNumber forknum);
extern void smgrsetowner(SMgrRelation *owner, SMgrRelation reln); extern void smgrsetowner(SMgrRelation *owner, SMgrRelation reln);
extern void smgrclose(SMgrRelation reln); extern void smgrclose(SMgrRelation reln);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment