Alexander Lakhin <exclusion@gmail.com> writes:
>> The regression tests on Windows sometimes fail with 'Permission denied'
>> errors. For example:
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory&dt=2019-12-11%2007%3A45%3A33
> I see two ways to fix the issue:
> 1) Unlink postmaster.pid using rename operation (adopt the solution from
> https://stackoverflow.com/questions/3764072/).
> 2) Ignore such 'Permission denied' error and just try to open the file
> once again (attached fix_open_for_unlink.patch implements this).
> I'm inclined to the second approach as pgwin32_open() already handles
> two transient states (Windows-only), and it could be useful not only for
> postmaster.pid, but for some other files.
Agreed that (2) seems like the way to go. However, I'm not too pleased
with the patch as given, because it is gratuitously different in almost
every possible way from the adjacent code that's doing just about the same
thing for those other transient failures. Why is the timeout duration
different? Why is the looping logic not identical? Why did you make a
different decision about whether logging might be a good idea? Actually,
why didn't you just extend the existing if-block to also cover this case?
Maybe there's good reasons to be different, but you didn't explain them.
I'm also not that excited about memorializing a stackoverflow discussion
as the reason to do something. Can we point to something in Microsoft's
official docs?
regards, tom lane
