From: | Satoshi Nagayasu <snaga(at)uptime(dot)jp> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Tomas Vondra <tv(at)fuzzy(dot)cz>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: New statistics for WAL buffer dirty writes |
Date: | 2013-09-07 03:32:34 |
Message-ID: | 522A9E52.1080804@uptime.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
The revised patch for wal buffer statistics is attached.
A test script is also attached. Please take a look.
Regards,
(2013/07/19 7:49), Satoshi Nagayasu wrote:
> 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(at)uptime(dot)jp>
Uptime Technologies, LLC. http://www.uptime.jp
Attachment | Content-Type | Size |
---|---|---|
xlogdirtywrite_v5.diff | text/plain | 22.3 KB |
test_xlogdirtywrite.sh | text/plain | 1.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2013-09-07 05:34:49 | Re: [PERFORM] encouraging index-only scans |
Previous Message | Kohei KaiGai | 2013-09-07 03:31:22 | Re: Custom Plan node |