Re: New statistics for WAL buffer dirty writes

On 29.10.2012 04:58, Satoshi Nagayasu wrote:
> 2012/10/24 1:12, Alvaro Herrera wrote:
>> Satoshi Nagayasu escribi�:
>>
>>> With this patch, walwriter process and each backend process
>>> would sum up dirty writes, and send it to the stat collector.
>>> So, the value could be saved in the stat file, and could be
>>> kept on restarting.
>>>
>>> The statistics could be retreive with using
>>> pg_stat_get_xlog_dirty_writes() function, and could be reset
>>> with calling pg_stat_reset_shared('walwriter').
>>>
>>> Now, I have one concern.
>>>
>>> The reset time could be captured in globalStats.stat_reset_timestamp,
>>> but this value is the same with the bgwriter one.
>>>
>>> So, once pg_stat_reset_shared('walwriter') is called,
>>> stats_reset column in pg_stat_bgwriter does represent
>>> the reset time for walwriter, not for bgwriter.
>>>
>>> How should we handle this?  Should we split this value?
>>> And should we have new system view for walwriter?
>>
>> I think the answer to the two last questions is yes.  It doesn't seem to
>> make sense, to me, to have a single reset timings for what are
>> effectively two separate things.
>>
>> Please submit an updated patch to next CF.  I'm marking this one
>> returned with feedback.  Thanks.
>>
>
> 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.

I've done a quick review of the v4 patch:

1) applies fine on HEAD, compiles fine

2) "make installcheck" fails because of a difference in the 'rules'
   test suite (there's a new view "pg_stat_walwriter" - see the
   attached patch for a fixed version or expected/rules.out)

3) I do agree with Alvaro that using the same struct for two separate
   components (bgwriter and walwriter) seems a bit awkward. For example
   you need to have two separate stat_reset fields, the reset code
   becomes much more verbose (because you need to list individual
   fields) etc.

   So I'd vote to either split this into two structures or keeping it
   as a single structure (although with two views on top of it).

4) Are there any other fields that might be interesting? Right now
   there's just "dirty_writes" but I guess there are other values. E.g.
   how much data was actually written etc.?

Tomas