(2012/11/27 7:42), Alvaro Herrera wrote:
> Satoshi Nagayasu escribió:
>
>> I attached the latest one, which splits the reset_time
>> for bgwriter and walwriter, and provides new system view,
>> called pg_stat_walwriter, to show the dirty write counter
>> and the reset time.
>
> Thanks. I gave this a look and I have a couple of comments:
>
> 1. The counter is called dirty_write. I imagine that this comes
> directly from the name of the nearby DTrace probe,
> TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_START. That probe comes from
> email 494C1565.3060105@sun.com committed in 4ee79fd20d9a. But there
> wasn't much discussion about the name back then. Maybe that was fine at
> the time because it was pretty much an internal matter, being so deep in
> the code. But we're now going to expose it to regular users, so we'd
> better choose a very good name because we're going to be stuck with it
> for a very long time. And the name "dirty" doesn't ring with me too
> well; what matters here is not that we're writing a buffer that is
> dirty, but the fact that we're writing while holding the WalInsertLock,
> so the name should convey the fact that this is a "locked" write or
> something like that. Or at least that's how I see the issue. Note the
> documentation wording:
>
> + <entry><structfield>dirty_writes</></entry>
> + <entry><type>bigint</type></entry>
> + <entry>Number of dirty writes, which means flushing wal buffers
> + because of its full.</entry>
Yes, "dirty_writes" came from the probe name, and if it needs to be
changed, "buffers_flush" would make sense for me in this situation,
because this counter is intended to show WAL writes due to "wal buffer
full".
> 2. Should we put bgwriter and walwriter data in separate structs? I
> don't think this is necessary, but maybe someone opines differently?
I tried to minimize an impact of this patch, but if I can change this
struct, yes, I'd like to split into two structs.
> 3.
> +/*
> + * WalWriter global statistics counter.
> + * Despite its name, this counter is actually used not only in walwriter,
> + * but also in each backend process to sum up xlog dirty writes.
> + * Those processes would increment this counter in each XLogWrite call,
> + * then send it to the stat collector process.
> + */
> +PgStat_MsgWalWriter WalWriterStats;
>
> Maybe we should use a different name for the struct, to avoid having to
> excuse ourselves for the name being wrong ...
Ok. How about WalBufferStats? I think this name could be accepted in
both the wal writer and each backend process.
--
Satoshi Nagayasu <snaga@uptime.jp>
Uptime Technologies, LLC. http://www.uptime.jp