On Sat, Oct 15, 2022 at 02:10:26PM -0700, Nathan Bossart wrote: > On Sat, Oct 15, 2022 at 10:19:05AM +0530, Bharath Rupireddy wrote: >> Can you please help me understand how name collisions can happen with >> temp file names including WAL file name, timestamp to millisecond >> scale, and PID? Having the timestamp is enough to provide a non-unique >> temp file name when PID wraparound occurs, right? Am I missing >> something here? > > Outside of contrived cases involving multiple servers, inaccurate clocks, > PID reuse, etc., it seems unlikely.
With a name based on a PID in a world where pid_max can be larger than the default and a timestamp, I would say even more unlikely than what you are implying with unlikely ;p > I think the right way to do this would be to add handling for leftover > files in the sigsetjmp() block and a shutdown callback (which just sets up > a before_shmem_exit callback). While this should ensure those files are > cleaned up after an ERROR or FATAL, crashes and unlink() failures could > still leave files behind. We'd probably also need to avoid cleaning up the > temp file if copy_file() fails because it already exists, as we won't know > if it's actually ours. Overall, I suspect it'd be more trouble than it's > worth. Agreed. My opinion is that we should keep basic_archive as minimalistic as we can: short still useful. It does not have to be perfect, just to fit with what we want it to show, as a reference. Anyway, the maths were wrong, so I have applied the patch of upthread, with an extra pair of parenthesis, a comment where epoch is declared to tell that it is in milliseconds, and a comment in basic_archive's Makefile to mention the reason why we have NO_INSTALLCHECK. -- Michael
signature.asc
Description: PGP signature