Simon Riggs wrote: > > On Wed, 2008-09-17 at 16:25 -0400, Alvaro Herrera wrote: > > > We've been profiling a large system (8 CPUs, 64 GB of memory, some > > dozens of disks) which seems rather more swamped than it should. Part > > of the problem seems to come from CleanupTempFiles, the second entry in > > oprofile output. > > I'm glad you've observed this also. I saw it about two years ago but > wasn't able to convince anyone else it existed at the time.
I couldn't find it in the archives. > Simple solution is to have a state variable so you can see whether a > backend has created an temp files in this transaction. Most don't, so I > think the above two solutions are overkill. If we created any, scan for > them, if not, don't. Just a simple boolean state, just as we have for > AtEOXact_RelationCache(). Ah -- like this? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/storage/file/fd.c =================================================================== RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/file/fd.c,v retrieving revision 1.143 diff -c -p -r1.143 fd.c *** src/backend/storage/file/fd.c 1 Jan 2008 19:45:51 -0000 1.143 --- src/backend/storage/file/fd.c 17 Sep 2008 21:33:28 -0000 *************** static int max_safe_fds = 32; /* default *** 121,126 **** --- 121,132 ---- #define FD_TEMPORARY (1 << 0) /* T = delete when closed */ #define FD_XACT_TEMPORARY (1 << 1) /* T = delete at eoXact */ + /* + * Flag to tell whether it's worth scanning VfdCache looking for temp files to + * close + */ + static bool any_temporary_files = false; + typedef struct vfd { signed short fd; /* current FD, or VFD_CLOSED if none */ *************** OpenTemporaryFile(bool interXact) *** 889,894 **** --- 895,903 ---- { VfdCache[file].fdstate |= FD_XACT_TEMPORARY; VfdCache[file].create_subid = GetCurrentSubTransactionId(); + + /* ensure cleanup happens at eoxact */ + any_temporary_files = true; } return file; *************** AtEOSubXact_Files(bool isCommit, SubTran *** 1603,1609 **** { Index i; ! if (SizeVfdCache > 0) { Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ for (i = 1; i < SizeVfdCache; i++) --- 1612,1618 ---- { Index i; ! if (SizeVfdCache > 0 && any_temporary_files) { Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ for (i = 1; i < SizeVfdCache; i++) *************** CleanupTempFiles(bool isProcExit) *** 1679,1685 **** { Index i; ! if (SizeVfdCache > 0) { Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ for (i = 1; i < SizeVfdCache; i++) --- 1688,1694 ---- { Index i; ! if (SizeVfdCache > 0 && (isProcExit || any_temporary_files)) { Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ for (i = 1; i < SizeVfdCache; i++) *************** CleanupTempFiles(bool isProcExit) *** 1697,1702 **** --- 1706,1713 ---- FileClose(i); } } + + any_temporary_files = false; } while (numAllocatedDescs > 0)
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers