Re: [PATCHES] bgwriter stats
On Thu, 29 Mar 2007, Magnus Hagander wrote: I've included a couple of more counters per ideas from Greg Smith in his logging patch. I just submitted a patch that logs the remaining things of value from my original that couldn't be derived from the information you're collecting. Between them I'm happy that a lot of previously hidden performance issues can now be monitored--not necessarily easily, but it's at least possible. I made one small change to your code in there as well I wanted to highlight here. You updated the buffers written by checkpoints one at a time as they wrote out. When I tried to develop something that monitored pg_stat_bgwriter looking for when checkpoints happened, this made it difficult to answer the important question how many buffers did the last checkpoint write? just from the stats structure because I assumed it's possible to get a view in the middle of the checkpoint. It took watching both the total and the checkpoint count, and that was hard to work with. I modified things so that the checkpoint buffers written number gets updated in one shot when the buffer flush is done. No partial results, much easier to monitor: when the buffers_checkpoint value changes, the difference from the previous value is what the last checkpoint wrote. I needed that total anyway which is why I just slipped it into the other patch. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] bgwriter stats
Greg Smith wrote: On Thu, 29 Mar 2007, Magnus Hagander wrote: I've included a couple of more counters per ideas from Greg Smith in his logging patch. I just submitted a patch that logs the remaining things of value from my original that couldn't be derived from the information you're collecting. Between them I'm happy that a lot of previously hidden performance issues can now be monitored--not necessarily easily, but it's at least possible. I made one small change to your code in there as well I wanted to highlight here. You updated the buffers written by checkpoints one at a time as they wrote out. When I tried to develop something that monitored pg_stat_bgwriter looking for when checkpoints happened, this made it difficult to answer the important question how many buffers did the last checkpoint write? just from the stats structure because I assumed it's possible to get a view in the middle of the checkpoint. It took watching both the total and the checkpoint count, and that was hard to work with. Uh, what? ;) The data in pg_stat_bgwriter certainly doesn't update *during* a checkpoint, if that's what you're saying. It is collected one by one in the global struct in the bgwriter, but it's only sent off to the stats collector once the checkpoint has completed (it's sent at the end of the loop in BackgroundWriterMain). And it's the data that's in the stats collector that you're looking at with pg_stat_bgwriter. //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] bgwriter stats
On Sun, 1 Apr 2007, Magnus Hagander wrote: The data in pg_stat_bgwriter certainly doesn't update *during* a checkpoint, if that's what you're saying. Scratch previous message, replace with dude ur code rulez! I was working on the assumption it was possible to get a partial result because I haven't had enough time track all the code paths involved to prove otherwise. Glad to hear it was never an issue. Doesn't change what I submitted though. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] bgwriter stats
Magnus Hagander [EMAIL PROTECTED] writes: Attached is a new version of the bgwriter stats patch. Per previous discussion, now uses the stats system only. Introduces a new stats message for bgwriter, and also introduces a global stats part of the stats file for statistics not bound to a database or table. It looks sane in a quick once-over, but the comments need some love --- too many typos and misspellings for my taste. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] bgwriter stats
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Attached is a new version of the bgwriter stats patch. Per previous discussion, now uses the stats system only. Introduces a new stats message for bgwriter, and also introduces a global stats part of the stats file for statistics not bound to a database or table. It looks sane in a quick once-over, but the comments need some love --- too many typos and misspellings for my taste. Ok. I'll be sure to work them over before I commit. Thanks for the quick check. //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] bgwriter stats
Magnus Hagander [EMAIL PROTECTED] writes: This seems quite a bizarre way to do things. Why wouldn't you implement this functionality by shipping messages to the stats collector? Would you suggest doing the same with the checkpoint counter, that's already in shared mem? I want to expose that number as well.. The shared-mem checkpoint counters serve an entirely different purpose: they're there to let backends detect when their requested checkpoint has been completed. They're not intended to count checkpoints over any long term (remember that sig_atomic_t need only be 8 bits wide). If you want to track stats about how many checkpoints have been done, I think that's a job for the stats collector. regards, tom lane ---(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
Re: [PATCHES] bgwriter stats
Magnus Hagander wrote: Anyway. Attached patch adds this to the bgwriter shared memory. Is it safe to do this, and then just have a regular function running in a normal backend pulling out the value and returning it to the user, without locking? If the variable is an int64, I don't believe so: the architecture might not implement atomic read/writes of int64 values. -Neil ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] bgwriter stats
Neil Conway wrote: Magnus Hagander wrote: Anyway. Attached patch adds this to the bgwriter shared memory. Is it safe to do this, and then just have a regular function running in a normal backend pulling out the value and returning it to the user, without locking? If the variable is an int64, I don't believe so: the architecture might not implement atomic read/writes of int64 values. Ok. But it should be safe if it's int32? Actually, since it's just statistics data, it wouldn't be a problem that it's not atomic, I think. If we really unlucky, we'll get the wrong value once. But most systems that poll such statistics can deal with that, in my experience. Then again, a normal int shouldn't be a problem either. //Magnus ---(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
Re: [PATCHES] bgwriter stats
Magnus Hagander wrote: Ok. But it should be safe if it's int32? You should probably use sig_atomic_t, to be safe. Although I believe that read/writes to int are atomic on most platforms, in any case. Actually, since it's just statistics data, it wouldn't be a problem that it's not atomic, I think. If we really unlucky, we'll get the wrong value once. I don't think that's the right attitude to take, at all. Why not just use a lock? It's not like the overhead will be noticeable. Alternatively, you can get a consistent read from an int64 variable using a sig_atomic_t counter, with a little thought. Off the top of my head, something like the following should work: have the writer increment the sig_atomic_t counter, adjust the int64 stats value, and then increment the sig_atomic_t again. Have the reader save a local copy of the sig_atomic_t counter aside, then read from the int64 counter, and then recheck the sig_atomic_t counter. Repeat until the local pre-read and post-read snapshots of the sig_atomic_t counter are identical. -Neil ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] bgwriter stats
Neil Conway wrote: Magnus Hagander wrote: Ok. But it should be safe if it's int32? You should probably use sig_atomic_t, to be safe. Although I believe that read/writes to int are atomic on most platforms, in any case. Ok. That's an easy enough change. Actually, since it's just statistics data, it wouldn't be a problem that it's not atomic, I think. If we really unlucky, we'll get the wrong value once. I don't think that's the right attitude to take, at all. Why not just use a lock? It's not like the overhead will be noticeable. Probably, but none of the other code appears to take a lock out on it :) Alternatively, you can get a consistent read from an int64 variable using a sig_atomic_t counter, with a little thought. Off the top of my head, something like the following should work: have the writer increment the sig_atomic_t counter, adjust the int64 stats value, and then increment the sig_atomic_t again. Have the reader save a local copy of the sig_atomic_t counter aside, then read from the int64 counter, and then recheck the sig_atomic_t counter. Repeat until the local pre-read and post-read snapshots of the sig_atomic_t counter are identical. Thinking more about it, I think that's unnecessary. 32 bits is quite enough - if you're graphing it (for example), those tools deal with wraps already. They're usually mdae to deal with things like number of bytes on a router interface, which is certainly 32 bit a lot faster than us. But I'll take note of that for some time when I actually *need* a 64-bit value.- //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] bgwriter stats
Magnus Hagander [EMAIL PROTECTED] writes: I want to be able to pull some stats out of the bgwriter to be able to track things. One thing is the total number of buffers written out. Other things are the number of checkpoints and such. Anyway. Attached patch adds this to the bgwriter shared memory. Is it safe to do this, and then just have a regular function running in a normal backend pulling out the value and returning it to the user, without locking? Given that only the bgwriter can write to it? This seems quite a bizarre way to do things. Why wouldn't you implement this functionality by shipping messages to the stats collector? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] bgwriter stats
Magnus Hagander [EMAIL PROTECTED] writes: Neil Conway wrote: I don't think that's the right attitude to take, at all. Why not just use a lock? It's not like the overhead will be noticeable. Probably, but none of the other code appears to take a lock out on it :) Huh? It doesn't use a lock for touching the checkpoint counters, but that's OK because they're sig_atomic_t. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] bgwriter stats
On Monday 19 March 2007 15:32, Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: I want to be able to pull some stats out of the bgwriter to be able to track things. One thing is the total number of buffers written out. Other things are the number of checkpoints and such. Anyway. Attached patch adds this to the bgwriter shared memory. Is it safe to do this, and then just have a regular function running in a normal backend pulling out the value and returning it to the user, without locking? Given that only the bgwriter can write to it? This seems quite a bizarre way to do things. Why wouldn't you implement this functionality by shipping messages to the stats collector? I'm with Tom on this one.. All of our current stats are done via the stats collector, we should continue that way. While we are on the subject of stats, does anybody else feel there is merrit in haveing block level writes tracked on a relation by relation bases? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] bgwriter stats
Tom Lane wrote: This seems quite a bizarre way to do things. Why wouldn't you implement this functionality by shipping messages to the stats collector? Would that have any benefits over the shmem approach? -Neil ---(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
Re: [PATCHES] bgwriter stats
Neil Conway [EMAIL PROTECTED] writes: Tom Lane wrote: This seems quite a bizarre way to do things. Why wouldn't you implement this functionality by shipping messages to the stats collector? Would that have any benefits over the shmem approach? Well, for one thing, it would fit naturally into the existing stats structure instead of being a wart on the side. The problem of atomic access to an int64 would go away, yet we'd still be able to keep a running int64 total of the reports. You wouldn't lose the total over a shutdown/restart. The value would obey the transactional-snapshot rules we've established for stats output, making it safe to try to correlate it with other stats. Probably a few other things I'm not thinking of... regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
