Re: [PATCH] Support for pg_stat_archiver view

On Sat, Jan 25, 2014 at 3:19 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sat, Jan 25, 2014 at 5:41 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> On Thu, Jan 23, 2014 at 4:10 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> On Thu, Jan 9, 2014 at 6:36 AM, Gabriele Bartolini
>>> <gabriele.bartolini@2ndquadrant.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