From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Lakhin <exclusion@gmail.com>
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)
In-reply-to: <c4c0e925-7b00-a555-9c56-80573af5ff4a@gmail.com>
References: <16161-7a985d2f1bbe8f71@postgresql.org>
 <4d28c7e1-fe91-b9fd-3227-478e549b05d5@gmail.com>
 <31098.1576445200@sss.pgh.pa.us>
 <c4c0e925-7b00-a555-9c56-80573af5ff4a@gmail.com>
Comments: In-reply-to Alexander Lakhin <exclusion@gmail.com>
	message dated "Mon, 16 Dec 2019 08:00:00 +0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <18123.1576527433.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Mon, 16 Dec 2019 15:17:13 -0500
Message-ID: <18124.1576527433@sss.pgh.pa.us>
Precedence: bulk

Alexander Lakhin <exclusion@gmail.com> writes:
> 16.12.2019 0:26, Tom Lane wrote:
>> 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.

> Second, I wanted to decrease a delay, as whole unlink operation
> (comprising of three kernel calls: CreateFile,
> SetDispositionInformationFile, CloseFile) performed for me in 0.0002
> seconds (as ProcessMonitor showed for several runs). Assuming even
> hundredfold duration seems sufficient.

Fair enough.  I wonder btw whether the 100ms wait quantum isn't
ridiculously long for modern machines.  We'd have to change it for
both conditions though to avoid odd behavior (or use two separate
wait counters?), so I didn't mess with that for now.

> Third, the logic of the existing loop confused me — I haven't seen a
> reason to sleep for a last time and then return an error without
> rechecking a condition.

That is pretty bogus now that you mention it, but the answer is to fix
it not just avert your eyes.

> And I think that the logging is irrelevant as we want to handle here
> just the DELETE_PENDING state.

True, if we're not going to wait very long then there's little need
to log.

I've pushed this with adjustment of the other loop and some fooling
with the comment --- I still don't see a need to cite stackoverflow
as an authority.

			regards, tom lane