Re: [pgsql-patches] [PATCHES] Patch to log usage of temporary files

2007-01-12 Thread Guillaume Smet
Hi all, Sorry for arriving so late into the discussion. I don't know if it's possible but it could be useful to have the text of the query which required the creation of the temporary files as an additional DETAIL line. At least, if it's possible to have it in this part of the code. Thoughts?

Re: [HACKERS] [pgsql-patches] [PATCHES] Patch to log usage of temporary files

2007-01-12 Thread Tom Lane
Guillaume Smet [EMAIL PROTECTED] writes: On 1/12/07, Tom Lane [EMAIL PROTECTED] wrote: (2) there is already a generalized solution to this, it's called log_min_error_statement. I didn't think of that when posting my message but Bruce seems to say that we can't use it in this case. Dunno why

Re: [pgsql-patches] [PATCHES] Patch to log usage of temporary files

2007-01-12 Thread Guillaume Smet
On 1/12/07, Bruce Momjian [EMAIL PROTECTED] wrote: Usually people don't want th query unless they ask for it. One nify trick would be to print the query as DETAIL unless they are already logging queries, but that just seems too complex. If you want the query, why not just log them all?

Re: [pgsql-patches] [PATCHES] Patch to log usage of temporary files

2007-01-12 Thread Bill Moran
In response to Guillaume Smet [EMAIL PROTECTED]: On 1/12/07, Bruce Momjian [EMAIL PROTECTED] wrote: Usually people don't want th query unless they ask for it. One nify trick would be to print the query as DETAIL unless they are already logging queries, but that just seems too complex. If

Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-11 Thread Simon Riggs
On Tue, 2007-01-09 at 17:16 -0500, Bruce Momjian wrote: Tom Lane wrote: /* reset flag so that die() interrupt won't cause problems */ vfdP-fdstate = ~FD_TEMPORARY; + PG_TRACE1(temp__file__cleanup, vfdP-fileName); +

Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-11 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes: Tom Lane wrote: The TRACE is in the wrong place no? I thought it was going to be after the stat() operation so it could pass the file size. We had that discussion already. If you only pass it after the stat() then you cannot use DTrace, except when you

Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-11 Thread Bruce Momjian
Simon Riggs wrote: Also, I dunno much about DTrace, but I had the idea that you can't simply throw a PG_TRACE macro into the source and think you are done --- isn't there a file of probe declarations to add to? Not to mention the documentation of what probes exist. I didn't like

Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-11 Thread Jim C. Nasby
On Thu, Jan 11, 2007 at 12:35:25PM -0500, Tom Lane wrote: I think the real criterion has to be is this probe useful to developers?. I'm entirely uninterested in adding probes that are targeted towards DBAs, as this one would have been --- if we think there's a problem that a DBA would have,

Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-11 Thread Simon Riggs
On Thu, 2007-01-11 at 12:37 -0500, Bruce Momjian wrote: The trace probe was incorrect Yes, incomplete, no doubt. On that point you were 100% right to reject. and kind of at an odd place. I don't think we want to go down the road of throwing trace in everwhere, do we? I would like to see a

Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-11 Thread Simon Riggs
On Thu, 2007-01-11 at 12:35 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Tom Lane wrote: The TRACE is in the wrong place no? I thought it was going to be after the stat() operation so it could pass the file size. We had that discussion already. If you only pass it

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Bill Moran
In response to Bruce Momjian [EMAIL PROTECTED]: I have applied a modified version of your patch. I renamed the parameter to 'log_temp_files', for consistency, added documentation, and improved the wording, particularly mentioning that the logging happens at file deletion time. Thanks. --

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Bruce Momjian
Bill Moran wrote: In response to Tom Lane [EMAIL PROTECTED]: Bill Moran [EMAIL PROTECTED] writes: Andrew Dunstan [EMAIL PROTECTED] wrote: Might be more robust to say if (trace_temp_files = 0) I specified in the GUC config that minimum allowable value is -1. I'd still tend

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes: + A value of zero logs all temporary files, and positive + values log only files whose size is equal or greater than + the specified number of bytes. Surely the measurement unit should be kbytes or disk blocks. And why aren't you

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: + A value of zero logs all temporary files, and positive + values log only files whose size is equal or greater than + the specified number of bytes. Surely the measurement unit should be kbytes or disk

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: Surely the measurement unit should be kbytes or disk blocks. And why aren't you using that GUC UNITS infrastructure Peter put in? Agreed. I have applied the following patch to make it kilobytes, and documented it. I didn't put '-1kB'

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: Surely the measurement unit should be kbytes or disk blocks. And why aren't you using that GUC UNITS infrastructure Peter put in? Agreed. I have applied the following patch to make it kilobytes, and documented

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Bill Moran
In response to Tom Lane [EMAIL PROTECTED]: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: Surely the measurement unit should be kbytes or disk blocks. And why aren't you using that GUC UNITS infrastructure Peter put in? Agreed. I have applied the following patch to make it

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Tom Lane
Bill Moran [EMAIL PROTECTED] writes: In response to Tom Lane [EMAIL PROTECTED]: and then zero can be the off position, and we need not worry about whether -1 is -1 byte or -1 kbyte. All doing this does is make it impossible to log temp files of 1 byte. How you figure that? It would make it

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Tom Lane
Bill Moran [EMAIL PROTECTED] writes: In response to Tom Lane [EMAIL PROTECTED]: Hmm, that could be a little bit ugly. Suggestion: redefine the value such that files *greater than* the given size are logged, It already is that way, with 0 effectively meaning log all. Oh, never mind,

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-09 Thread Bruce Momjian
Tom Lane wrote: Bill Moran [EMAIL PROTECTED] writes: In response to Tom Lane [EMAIL PROTECTED]: and then zero can be the off position, and we need not worry about whether -1 is -1 byte or -1 kbyte. All doing this does is make it impossible to log temp files of 1 byte. How you

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-05 Thread Jim Nasby
On Jan 3, 2007, at 4:20 PM, Bill Moran wrote: * trace_temp_files is now an int: -1 disables, 0 and up equate to log if the file is this size or larger Another thought is to allow ignoring files over a certain size. The reason is that if you end up creating 10MB of temp files, you can

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-04 Thread Bill Moran
In response to Andrew Dunstan [EMAIL PROTECTED]: Bill Moran wrote: Andrew Dunstan [EMAIL PROTECTED] wrote: Bill Moran wrote: +if (trace_temp_files != -1) Might be more robust to say if (trace_temp_files = 0) Because it would allow for the easy addition

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-04 Thread Andrew Dunstan
Bill Moran wrote: In response to Andrew Dunstan [EMAIL PROTECTED]: Bill Moran wrote: Andrew Dunstan [EMAIL PROTECTED] wrote: Bill Moran wrote: + if (trace_temp_files != -1) Might be more robust to say if (trace_temp_files = 0)

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-04 Thread Tom Lane
Bill Moran [EMAIL PROTECTED] writes: Andrew Dunstan [EMAIL PROTECTED] wrote: Might be more robust to say if (trace_temp_files = 0) I specified in the GUC config that minimum allowable value is -1. I'd still tend to go with Andrew's suggestion because it makes this particular bit of code

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-04 Thread Bill Moran
In response to Tom Lane [EMAIL PROTECTED]: Bill Moran [EMAIL PROTECTED] writes: Andrew Dunstan [EMAIL PROTECTED] wrote: Might be more robust to say if (trace_temp_files = 0) I specified in the GUC config that minimum allowable value is -1. I'd still tend to go with Andrew's

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-03 Thread Simon Riggs
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

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-03 Thread Andrew Dunstan
Bill Moran wrote: + if (trace_temp_files != -1) Might be more robust to say if (trace_temp_files = 0) cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-03 Thread Bill Moran
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

Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

2007-01-03 Thread Bill Moran
Andrew Dunstan [EMAIL PROTECTED] wrote: Bill Moran wrote: + if (trace_temp_files != -1) Might be more robust to say if (trace_temp_files = 0) Because it would allow for the easy addition of more negative numbers with magic value? ---(end of

[PATCHES] Patch to log usage of temporary files

2007-01-02 Thread Bill Moran
Thanks to Simon Riggs and Bruce for input that helped me put this together. -- 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 Tue

Re: [PATCHES] Patch to log usage of temporary files

2007-01-02 Thread Alvaro Herrera
Bill Moran wrote: Thanks to Simon Riggs and Bruce for input that helped me put this together. Please change things to save the stat() syscall when the feature is not in use. Nitpick: also note our brace placement convention (though this would be fixed by pgindent, but still). -- Alvaro

Re: [PATCHES] Patch to log usage of temporary files

2007-01-02 Thread Andrew Dunstan
Bill Moran wrote: + if (stat(vfdP-fileName, filestats) == 0) { + if (trace_temp_files) Shouldn't these tests be the other way around? cheers andrew ---(end of broadcast)--- TIP 7: You can help support the

Re: [PATCHES] Patch to log usage of temporary files

2007-01-02 Thread Bill Moran
In response to Alvaro Herrera [EMAIL PROTECTED]: Bill Moran wrote: Thanks to Simon Riggs and Bruce for input that helped me put this together. 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

Re: [PATCHES] Patch to log usage of temporary files

2007-01-02 Thread Tom Lane
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