On Wed, May 1, 2019 at 9:56 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
On Wed, May 1, 2019 at 12:48 AM Julien Rouhaud <rjuju123@gmail.com> wrote: > > On Tue, Apr 30, 2019 at 11:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > Julien Rouhaud <rjuju123@gmail.com> writes: > > > On Tue, Apr 30, 2019 at 6:33 PM Tomas Vondra > > > <tomas.vondra@2ndquadrant.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?