Re: New statistics for WAL buffer dirty writes

Will revise and re-resubmit for the next CF.

Regards,

2013/07/19 1:06, Alvaro Herrera wrote:
>
> What happened to this patch?  We were waiting on an updated version from
> you.
>
>
> Satoshi Nagayasu wrote:
>> (2012/12/10 3:06), Tomas Vondra wrote:
>>> 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:
>>
>> Thanks for the review, and sorry for my delayed response.
>>
>>> 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)
>>
>> Ah, I forgot about the regression test. I will fix it. Thanks.
>>
>>> 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).
>>
>> Ok, I will split it into two structs, PgStat_BgWriterGlobalStats and
>> PgStat_WalWriterGlobalStats, and will modify PgStat_GlobalStats to
>> hold those two structs in the stat collector.
>>
>>> 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.?
>>
>> AFAIK, I think those numbers can be obtained by calling
>> pg_current_xlog_insert_location() or pg_current_xlog_location(),
>> but if we need it, I will add it.
>>
>> Regards,
>
>


-- 
Satoshi Nagayasu <snaga@uptime.jp>
Uptime Technologies, LLC. http://www.uptime.jp