Re: Timing issue in pg_basebackup

On Wed, 21 Oct 2026 at 22:16, Jesper St John <jesper(at)stjohn(dot)se> wrote:
>
> On 2026-10-20 23:11, Kirill Reshke wrote:

> >
> >> I've included a patch that moves the replication slot handing to run
> >> before the backup part as a separate function.
> >>
> >> Some possible caveats already:
> >> * Since moving the slot creation, we might run in to trouble if for
> >> example, --checkpoint=spread is used since the slot is open for
> >> longer.
> >> * If any error arises in the backup part, now there will be a
> >> lingering
> >> slot when previously no slot was created.
> >
> > lingering slots looks like a real regression here. can we remove them
> > when basebackup client disconnects?
>
> A lingering slot could also happen today - only with this patch there's
> a higher risk of it happening.

Higher risk of lingering slots is also a regression. But looks like
this risk is anyway close to zero.

> Would some cleanup code in a function and use it with atexit() work?

Something like this should do, can be made a separate patch thought

> >
> > NIT: comment in PrepareLogStreamer loses its meaning after code motion:
> >
> > + /* Get a second connection */
> > + bgconn = GetConnection();
> >
>
> I've moved the code to its own function - the comment were there
> already.
>

Yep, the point is It looks very out-of-context here. Like, the first
line of the function says we're taking a second connection. Second to
what (to basebackup)? Maybe rephrase here as 'Get a wal-serving
connection' or something.

--
Best regards,
Kirill Reshke