In response to "Simon Riggs" <[EMAIL PROTECTED]>: > On Tue, 2007-01-02 at 18:20 -0500, Tom Lane wrote: > > Bill Moran <[EMAIL PROTECTED]> writes: > > > In response to Alvaro Herrera <[EMAIL PROTECTED]>: > > >> Please change things to save the stat() syscall when the feature is not > > >> in use. > > > > > Do you have a suggestion on how to do that and still have the PG_TRACE1() > > > work? That was specifically requested by Simon Riggs. > > > > Well, we are NOT paying a stat() call on every single file close, > > whether Simon wants it or not. > > Simon doesn't/wouldn't want the stat() call on each file close. > > If you put the PG_TRACE macro outside of the if test, yet prior to the > file close, you can pass the filename through like this > > PG_TRACE1(temp__file__cleanup, vfdP->fileName); > > That way DTrace can make its own call to find out filesize, if it would > like to... and we don't need to stat() before each temp file close. > That's much more flexible and useful, as well as better performance.
OK, I think I've managed to adjust this patch so that everyone is happy ;) and it's better as well. * PG_TRACE will work whether the GUC var is enabled or not, it sends the fileName, as suggested by Simon * stat() call is not made if trace_temp_files is disabled * trace_temp_files is now an int: -1 disables, 0 and up equate to "log if the file is this size or larger" * Cleaned things up a bit -- should be more in line with PostgreSQL coding standards * failed stat is reported as LOG instead of ERROR Done a bit of testing here, and everything seems to be in order. -- Bill Moran Collaborative Fusion Inc.
diff -c -r src.orig/backend/storage/file/fd.c src/backend/storage/file/fd.c *** src.orig/backend/storage/file/fd.c Thu Dec 7 15:44:42 2006 --- src/backend/storage/file/fd.c Wed Jan 3 15:05:54 2007 *************** *** 50,55 **** --- 50,56 ---- #include "access/xact.h" #include "storage/fd.h" #include "storage/ipc.h" + #include "utils/guc.h" /* *************** *** 938,944 **** void FileClose(File file) { ! Vfd *vfdP; Assert(FileIsValid(file)); --- 939,946 ---- void FileClose(File file) { ! Vfd *vfdP; ! struct stat filestats; Assert(FileIsValid(file)); *************** *** 968,973 **** --- 970,992 ---- { /* reset flag so that die() interrupt won't cause problems */ vfdP->fdstate &= ~FD_TEMPORARY; + PG_TRACE1(temp__file__cleanup, vfdP->fileName); + if (trace_temp_files != -1) + { + if (stat(vfdP->fileName, &filestats) == 0) + { + if (filestats.st_size >= trace_temp_files) + { + ereport(LOG, + (errmsg("temp file: size %lu path \"%s\"", + filestats.st_size, vfdP->fileName))); + } + } + else + { + elog(LOG, "Could not stat \"%s\": %m", vfdP->fileName); + } + } if (unlink(vfdP->fileName)) elog(LOG, "failed to unlink \"%s\": %m", vfdP->fileName); diff -c -r src.orig/backend/utils/misc/guc.c src/backend/utils/misc/guc.c *** src.orig/backend/utils/misc/guc.c Wed Nov 29 09:50:07 2006 --- src/backend/utils/misc/guc.c Wed Jan 3 13:51:14 2007 *************** *** 180,186 **** int log_min_messages = NOTICE; int client_min_messages = NOTICE; int log_min_duration_statement = -1; ! int num_temp_buffers = 1000; char *ConfigFileName; --- 180,187 ---- int log_min_messages = NOTICE; int client_min_messages = NOTICE; int log_min_duration_statement = -1; ! int trace_temp_files = -1; ! int num_temp_buffers = 1000; char *ConfigFileName; *************** *** 1471,1477 **** &log_min_duration_statement, -1, -1, INT_MAX / 1000, NULL, NULL }, ! { {"bgwriter_delay", PGC_SIGHUP, RESOURCES, gettext_noop("Background writer sleep time between rounds in milliseconds"), --- 1472,1478 ---- &log_min_duration_statement, -1, -1, INT_MAX / 1000, NULL, NULL }, ! { {"bgwriter_delay", PGC_SIGHUP, RESOURCES, gettext_noop("Background writer sleep time between rounds in milliseconds"), *************** *** 1657,1662 **** --- 1658,1673 ---- }, &server_version_num, PG_VERSION_NUM, PG_VERSION_NUM, PG_VERSION_NUM, NULL, NULL + }, + + { + {"trace_temp_files", PGC_USERSET, LOGGING_WHAT, + gettext_noop("Log the use of temp files larger than this size."), + gettext_noop("Size and location of each temp file is reported."), + NULL + }, + &trace_temp_files, + -1, -1, INT_MAX, NULL, NULL }, /* End-of-list marker */ diff -c -r src.orig/backend/utils/misc/postgresql.conf.sample src/backend/utils/misc/postgresql.conf.sample *** src.orig/backend/utils/misc/postgresql.conf.sample Mon Nov 20 20:23:37 2006 --- src/backend/utils/misc/postgresql.conf.sample Wed Jan 3 11:05:48 2007 *************** *** 333,338 **** --- 333,341 ---- #log_statement = 'none' # none, ddl, mod, all #log_hostname = off + #trace_temp_files = -1 # Log usage of temporary files larger than + # the specified size (in bytes). -1 disables. + # 0 effectively logs all temp file usage. #--------------------------------------------------------------------------- # RUNTIME STATISTICS diff -c -r src.orig/include/utils/guc.h src/include/utils/guc.h *** src.orig/include/utils/guc.h Thu Oct 19 14:32:47 2006 --- src/include/utils/guc.h Wed Jan 3 13:45:52 2007 *************** *** 123,128 **** --- 123,129 ---- extern int log_min_messages; extern int client_min_messages; extern int log_min_duration_statement; + extern int trace_temp_files; extern int num_temp_buffers;
---------------------------(end of broadcast)--------------------------- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match