Re: pgsql: Track block level checksum failures in pg_stat_database

On Wed, May 1, 2019 at 9:56 AM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:

> On Wed, May 1, 2019 at 12:48 AM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> >
> > On Tue, Apr 30, 2019 at 11:56 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >
> > > Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
> > > > On Tue, Apr 30, 2019 at 6:33 PM Tomas Vondra
> > > > <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
> > > >> It seems this commit forgot to add PgStat_MsgChecksumFailure to the
> > > >> PgStat_Msg union.
> > >
> > > > Oops, indeed. That's embarrassing. Trivial fix attached if that
> helps.
> > >
> > > Seems like this exposes a generic weakness in the way pgstat.c is
> coded.
> > > I'm inclined to adjust the switch logic in PgstatCollectorMain along
> > > the lines of
> > >
> > > - pgstat_recv_inquiry((PgStat_MsgInquiry *) &msg,
> len);
> > > + pgstat_recv_inquiry(&msg.msg_inquiry, len);
> > >
> > > so that you *couldn't* forget to extend the union, as long as you
> > > followed the existing code's style.
> >
> > I'll send an updated patch tomorrow
>
> Here's an updated version. It turns out that PgStat_MsgTempFile had
> also been forgotten and never noticed.
>

Looks good to me in general

I don't see the reasoning behind changing the name from msg_autovacuum to
msg_autovacuum_start anywhere, perhaps I missed a part of the discussion?
Was that a change intended to be part of it?

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