Re: Re: BUGFIX: standby disconnect can corrupt serialized reorderbuffers

Hi Craig,

On 1/21/18 5:45 PM, Craig Ringer wrote:
> On 6 January 2018 at 08:28, Alvaro Herrera <alvherre@alvh.no-ip.org
> <mailto:alvherre@alvh.no-ip.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,
-- 
-David
david@pgmasters.net