Re: Re: BUGFIX: standby disconnect can corrupt serialized reorder buffers

On 5 March 2018 at 23:25, David Steele <david(at)pgmasters(dot)net> wrote:

> Hi Craig,
>
> On 1/21/18 5:45 PM, Craig Ringer wrote:
> > On 6 January 2018 at 08:28, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org
> > <mailto:alvherre(at)alvh(dot)no-ip(dot)org>> wrote:
> >
> > I think this should use ReadDirExtended with an elevel less than
> ERROR,
> > and do nothing.
> >
> > Why have strcmp(.) and strcmp(..)? These are going to be skipped by
> the
> > comparison to "xid" prefix anyway. Looks like strcmp processing
> > power waste.
> >
> > Please don't use bare sprintf() -- upgrade to snprintf.
> >
> > With this coding, if I put a root-owned file "xidfoo" in a replslot
> > directory, it will PANIC the server. Is that okay? Why not read the
> > file name with sscanf(), since we know the precise format it has?
> Then
> > we don't need to bother with random crap left around. Maybe a good
> time
> > to put the "xid-%u-lsn-%X-%X.snap" literal in a macro? I guess the
> > rationale is that if you let random people put "xidfoo" files in your
> > replication slot dirs, you deserve a PANIC anyway, but I'm not sure.
> >
> > I'm happy to address those comments.
> >
> > The PANIC probably made sense when it was only done on startup, but not
> > now it's at runtime.
> >
> > The rest is mainly retained from the prior code, but it's a good chance
> > to make those changes.
> This patch was marked Waiting on Author last December. Do you know when
> you'll have a chance to provide an updated patch?
>
> Given that it's a bug fix it would be good to see a patch for this CF,
> or very soon after.
>
>
Thanks for the reminder. I'll address it today if I can.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services