On Sat, Jan 25, 2014 at 3:19 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sat, Jan 25, 2014 at 5:41 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Thu, Jan 23, 2014 at 4:10 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> On Thu, Jan 9, 2014 at 6:36 AM, Gabriele Bartolini
>>> <gabriele(dot)bartolini(at)2ndquadrant(dot)it> wrote:
>>>> Il 08/01/14 18:42, Simon Riggs ha scritto:
>>>>> Not sure I see why it needs to be an SRF. It only returns one row.
>>>> Attached is version 4, which removes management of SRF stages.
>>>
>>> I have been looking at v4 a bit more, and found a couple of small things:
>>> - a warning in pgstatfuncs.c
>>> - some typos and format fixing in the sgml docs
>>> - some corrections in a couple of comments
>>> - Fixed an error message related to pg_stat_reset_shared referring
>>> only to bgwriter and not the new option archiver. Here is how the new
>>> message looks:
>>> =# select pg_stat_reset_shared('popo');
>>> ERROR: 22023: unrecognized reset target: "popo"
>>> HINT: Target must be "bgwriter" or "archiver"
>>> A new version is attached containing those fixes. I played also with
>>> the patch and pgbench, simulating some archive failures and successes
>>> while running pgbench and the reports given by pg_stat_archiver were
>>> correct, so I am marking this patch as "Ready for committer".
>>
>> I refactored the patch further.
>>
>> * Remove ArchiverStats global variable to simplify pgarch.c.
>> * Remove stats_timestamp field from PgStat_ArchiverStats struct because
>> it's not required.
> Thanks, pgstat_send_archiver is cleaner now.
>
>>
>> I have some review comments:
>>
>> + s.archived_wals,
>> + s.last_archived_wal,
>> + s.last_archived_wal_time,
>> + s.failed_attempts,
>> + s.last_failed_wal,
>> + s.last_failed_wal_time,
>>
>> The column names of pg_stat_archiver look not consistent at least to me.
>> What about the followings?
>>
>> archive_count
>> last_archived_wal
>> last_archived_time
>> fail_count
>> last_failed_wal
>> last_failed_time
> And what about archived_count and failed_count instead of respectively
> archive_count and failed_count? The rest of the names are better now
> indeed.
Please find attached an updated patch updated with the new column
names (in context diffs this time).
Regards,
--
Michael