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

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

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

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

Magnus Hagander <magnus@hagander.net> 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

Magnus Hagander <magnus@hagander.net> 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

On Monday 19 March 2007 15:32, Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> 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?

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

Neil Conway <neilc@samurai.com> 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

"Magnus Hagander" <magnus@hagander.net> 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