Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files
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);
+ if (log_temp_files = 0)
+ {
+ if (stat(vfdP-fileName, filestats) == 0)
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 already get a message in the
log and therefore don't need DTrace. DTrace can get the filesize if it
likes, but thats up to the script author.
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 the macro in that area anyway. It seems too adhock to
just throw it in when we have so few places monitored now. Removed.
err... why are we removing it? The patch should have included an
addition to the probes.d file also, but that should be fixed, not
removed. Don't we normally reject incomplete patches?
You can't say we don't have many probes so we won't add one. There never
will be many if we do that - its a circular argument.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files
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 already get a message in the log and therefore don't need DTrace. We may have had the discussion, but apparently you didn't follow it :-(. The point of the proposal was that if you wanted to DTrace temp files you would set log_temp_files to some large value, thus causing the stat() call to occur but no log message to come out. This is surely a lot more efficient than having to have DTrace open the file for itself --- and if you don't care about micro-efficiency, what's wrong with using the logging option? You can't say we don't have many probes so we won't add one. There never will be many if we do that - its a circular argument. 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, we need to offer a more portable solution than that. Which we did, in the form of a logging option, which makes the DTrace probe pretty useless anyway. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files
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 the macro in that area anyway. It seems too adhock to just throw it in when we have so few places monitored now. Removed. err... why are we removing it? The patch should have included an addition to the probes.d file also, but that should be fixed, not removed. Don't we normally reject incomplete patches? You can't say we don't have many probes so we won't add one. There never will be many if we do that - its a circular argument. The trace probe was incorrect 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 more systematic approach to it. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files
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, we need to offer a more portable solution than that. Which we did, in the form of a logging option, which makes the DTrace probe pretty useless anyway. But the problem with just logging stuff is it's not a monitoring solution. Granted, it's better than nothing, but in a production environment I'd like to have some way to monitor temp file usage/utilization over time. AFAIK dtrace would provide that capability, thought I don't think it'd be unreasonable to have our own counter as well. Perhaps add temp_usage_count and temp_usage_size to pg_stat_database (number of times something spilled to disk and total size, respectively). -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files
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 more systematic approach to it. I guess my systematic approach was to add PG_TRACE to all new log points from now on, so we have a choice of which trace/log mechanism to use. I'm happy with everything but the portability of DTrace, however I'm fond of the idea of sowing seeds for the time when other solutions will work also. I've a bag of PG_TRACE trace points to add sometime soon. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [pgsql-patches] [HACKERS] [PATCHES] Patch to log usage of temporary files
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 after the stat() then you cannot use DTrace, except when you already get a message in the log and therefore don't need DTrace. We may have had the discussion, but apparently you didn't follow it :-(. My apologies. You can't say we don't have many probes so we won't add one. There never will be many if we do that - its a circular argument. 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, we need to offer a more portable solution than that. Which we did, in the form of a logging option, which makes the DTrace probe pretty useless anyway. Well, you know my major objection to including DTrace was all to do with portability. I'm happy that the way its been implemented allows other solutions to take advantage of the trace points also. We're working on 8.3 now and by the time that is delivered and perhaps for 2 years hence, i.e. Aug 2009, the software will be in production use. In that period, DTrace will have been ported more widely and I'm hearing that some kind of user space solution for Linux will mature in that time also. If that isn't true then I'll be more interested in some custom tracing solutions built around the PG_TRACE macro concept. My thought is to provide both a log-based trace solution as has been done, plus a hook for PG_TRACE (not just DTrace) at the same time. i.e. each time we enhance the logging infrastructure, take the time to place a trace point there also. Theologically, we both know we see things differently on the DBA v Developer discussion. The only point I would make is that the more information you give the DBA, the more comes back to you as a Developer. You, personally, could not possibly have interacted with as many server set-ups required to highlight the problems and issues you address. It's only because of the info provided by the existing system that you're able to make headway with rare optimizer problems. My perspective is that if you help the DBA you also help the Developer; if you help the Developer only, then the Developer's information is also inevitably restricted. The tip says EXPLAIN ANALYZE is your friend. It's right, and it isn't just talking to DBAs. My feeling is that this is true for all tools/trace mechanisms. I'd rather be sent the info than have to go do it myself on an individual basis. Indirect access isn't the best way, but we harvest a much wider range of information that way. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
