On 9/12/17 16:39, Michael Banck wrote: > We could split up the logic here and create the optional physical > replication slot in the main connection and the temporary one in the WAL > streamer connection, but this would keep any fragility around for > (likely more frequently used) temporary replication slots. It would make > the patch much smaller though if I revert touching temporary slots at > all.
That's what I was thinking.
But:
If the race condition concern that Jeff was describing is indeed correct, then the current use of temporary replication slots would be equally affected. So I think either we already have a problem, or we don't and then this patch wouldn't introduce a new one.
I don't know the details of this well enough.
It is possible the race condition is entirely theoretical, as the WAL files won't be eligible for recycling until the end of the *next* (future) checkpoint (for no good reason, as far as I can tell, although fortunate in this case) so that should give plenty of opportunity for the WAL streamer to connect in all but the weirdest cases. When it reserves the WAL, I assume it does so back-dating to the LSN position reported by pg_start_backup and will promptly fail if those are no longer available?
I don't want to hold up the patch based on theoretical questions when it solves an actual problem.