On Sat, Jul 28, 2012 at 3:33 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Sat, Jul 7, 2012 at 9:17 PM, Satoshi Nagayasu <snaga@uptime.jp> wrote:
>> Hi,
>>
>> Jeff Janes has pointed out that my previous patch could hold
>> a number of the dirty writes only in single local backend, and
>> it could not hold all over the cluster, because the counter
>> was allocated in the local process memory.
>>
>> That's true, and I have fixed it with moving the counter into
>> the shared memory, as a member of XLogCtlWrite, to keep total
>> dirty writes in the cluster.
>
...
> The comment "XLogCtrlWrite must be protected with WALWriteLock"
> mis-spells XLogCtlWrite.
>
> The final patch will need to add a sections to the documentation.
Thanks to Robert and Tom for addressing my concerns about the pointer
volatility.
I think there is enough consensus that this is useful without adding
more things to it, like histograms or high water marks.
However, I do think we will want to add a way to query for the time of
the last reset, as other monitoring features are going that way.
Is it OK that the count is reset upon a server restart?
pg_stat_bgwriter, for example, does not do that. Unfortunately I
think fixing this in an acceptable way will be harder than the entire
rest of the patch was.
The coding looks OK to me, it applies and builds, and passes make
check, and does what it says. I didn't do performance testing, as it
is hard to believe it would have a meaningful effect.
I'll marked it as waiting on author, for the documentation and reset
time. I'd ask a more senior hacker to comment on the durability over
restarts.
Cheers,
Jeff