Re: [PATCH] Support for pg_stat_archiver view

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Gabriele Bartolini <gabriele(dot)bartolini(at)2ndquadrant(dot)it>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Support for pg_stat_archiver view
Date: 2014-01-04 12:25:31
Message-ID: CABUevEzkbNd=b5Kbgm3ZpFKpM=MWb2+Y-POuH=EH2cdKksypjw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jan 4, 2014 at 1:33 AM, Gabriele Bartolini <
gabriele(dot)bartolini(at)2ndquadrant(dot)it> wrote:

> Hello,
>
> please find attached the patch that adds basic support for the
> pg_stat_archiver system view, which allows users that have continuous
> archiving procedures in place to keep track of some important metrics
> and information.
>
> Currently, pg_stat_archiver displays:
>
> * archived_wals: number of successfully archived WAL files since start
> (or the last reset)
> * last_archived_wal: last successfully archived WAL file
> * last_archived_wal_time: timestamp of the latest successful WAL archival
> * stats_reset: time of last stats reset
>
> This is an example of output:
>
> postgres=# select * from pg_stat_archiver ;
> -[ RECORD 1 ]----------+------------------------------
> archived_wals | 1
> last_archived_wal | 000000010000000000000001
> last_archived_wal_time | 2014-01-04 01:01:08.858648+01
> stats_reset | 2014-01-04 00:59:25.895034+01
>
> Similarly to pg_stat_bgwriter, it is possible to reset statistics just
> for this context, calling the pg_stat_reset_shared('archiver') function.
>
> The patch is here for discussion and has been prepared against HEAD.
> It includes also changes in the documentation and the rules.out test.
>
> I plan to add further information to the pg_stat_archiver view,
> including the number of failed attempts of archival and the WAL and
> timestamp of the latest failure. However, before proceeding, I'd like to
> get some feedback on this small patch as well as advice on possible
> regression tests to be added.
>
>
My first reaction was that exactly those two things were missing. And then
I read your whole email :)

With those two, I think it would make much sense to have a view like this.

I'd suggest making the view on top of an SRF like pg_stat_replication and
pg_stat_activity (for example), instead of a whole lot of separate function
calls like the older stats views.

in pgarch_ArchiveDone() you seem to be increasing the m_archived_vals value
for each call and then sending it off. And then you add that number in the
stats collector. Isn't that going to add the wrong number in the end -
after a while, the archiver is going to send "add 100" when it's just sent
one file? ISTM that pgstat_recv_archiver should just do ++ on the value?

Oh, and you need to change the format id number of the stats file.

There's a quick review you for ;) I think it's definitely worthwhile with
those things fixed (and a proper review, that was just a quick one-over)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ian Lawrence Barwick 2014-01-04 12:39:27 comment typo in postgres_fdw/postgres_fdw.c
Previous Message Emre Hasegeli 2014-01-04 12:19:49 Wrong comment for bitncmp function in network.c