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

Reply via email to