Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)

At Thu, 19 Dec 2019 07:00:00 +0300, Alexander Lakhin <exclusion(at)gmail(dot)com> wrote in
> 19.12.2019 4:39, Kyotaro Horiguchi wrote:
> > At Wed, 18 Dec 2019 17:09:15 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> >> Alexander Lakhin <exclusion(at)gmail(dot)com> writes:
> >>> So I see three ways now:
> >>> 1) Revert the pgwin32_open change and choose the previously rejected
> >>> approach: Unlink postmaster.pid using rename operation (adopt the
> >>> solution from https://stackoverflow.com/questions/3764072/).
> >>> 2) Add a check for directory in pgwin32_open, but I think then we're
> >>> getting close to checking there just for postmaster.pid.
> >>> 3) Find out how often we get ERROR_ACCESS_DENIED during all regression
> >>> tests and if such error count is near zero, then add the check for isdir
> >>> in fsync_fname_ext and friends, and get done.
> >> I'm not terribly thrilled with (1) because I do not think that
> >> postmaster.pid accesses are the only place where we have this issue.
> >> And (3) is not only not a forward-looking solution, but it imagines
> >> that the speed of the regression tests is the only thing we need to
> >> worry about here. We generally don't assume that the regression
> >> tests are an accurate model of the performance users will see.
> >>
> >> So that leaves us with (2), it seems.
> > Agreed for (2).
> >
> > I'm not sure about the point of the discussion that stat()'ing is
> > messy. If it's about performance, does stat()ing only when CreateFile
> > returned ERROR_ACCESS_DENIED helps? Anyway we wait for a moment in
> > that case.
> Please look at the patch that implements (2). It makes `vcregress
> recoverycheck` pass (and my demo restart test still passes too).
> As open(dir) is getting a little more expensive than before, maybe it's
> still worthwhile to patch fsync*(..., isdir,...). I can prepare such a
> patch if so.

Sorry, I hadn't looked it.

+#ifdef WIN32
+ /* Windows doesn't allow us to open directories at all.
+ */
+ if (isdir)
+ return 0;
+#endif

Doesn't it need to handle ENOENT or anything that can happen if
OpenTransientFile() were called?

And.. how do we regard about the crash-safetiness on Windows? After
some googling it seems that not only CreateFile but also fsync on
directories cannot be performed on Windows at all.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center