From: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-committers <pgsql-committers(at)postgresql(dot)org> |
Subject: | Re: pgsql: Add test for postmaster crash restarts. |
Date: | 2017-10-01 02:28:39 |
Message-ID: | 75777957-d16f-f799-d0ec-bc3d5b7c72de@2ndQuadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 09/30/2017 06:44 PM, Andres Freund wrote:
> On 2017-09-30 15:27:12 -0700, Andres Freund wrote:
>> On 2017-09-30 18:21:33 -0400, Andrew Dunstan wrote:
>>> [re-adding commiters which I inadvertently left off]
>>>
>>>
>>> On 09/30/2017 06:10 PM, Andres Freund wrote:
>>>>
>>>>> I was just looking at this. Why aren't we using "pg_ctl kill" to
>>>>> terminate the backend? That's supposed to be portable.
>>>> Because pg_ctl can't do that for any process but postmaster, no? The
>>>> test is supposed to find issues with backend death (and has
>>>> defficiencies in error reporting already, and would have caught a bug
>>>> I'd introduced previously).
>>> No, I don't think so. That's not what the docs say. That's why you give
>>> it a pid argument" "pg_ctl kill signal_name process_id"
>> Oh, cool. Didn't know that one. So the answer is:
>> "Because Andres doesn't know squat.".
>>
>> But even after fixing that, there unfortunately is:
>>
>> static void
>> set_sig(char *signame)
>> {
>> …
>> #if 0
>> /* probably should NOT provide SIGKILL */
>> else if (strcmp(signame, "KILL") == 0)
>> sig = SIGKILL;
>> #endif
>>
>> I'm unclear on what that provision is achieving? If you can kill with
>> pg_ctl you can do other nasty stuff too (like just use kill instead of
>> pg_ctl)?
I put it in when we rewrote pg_ctl in C many years ago, possibly out of
a superabundance of caution. I agree it's worth revisiting. I think the
idea was that there's a difference between an ordinary footgun and an
officially sanctioned footgun :-)
> Could you perhaps test whether windows likes things after the following
> patch? I don't think the kill_kill guarantees are really needed here,
> so we might even be able to allow this on msvc.
>
Haven't tested on MSVC but with this patch it passes on jacana (mingw).
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2017-10-01 02:32:14 | Re: pgsql: Add test for postmaster crash restarts. |
Previous Message | Peter Geoghegan | 2017-10-01 01:25:30 | Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2017-10-01 02:32:14 | Re: pgsql: Add test for postmaster crash restarts. |
Previous Message | Peter Geoghegan | 2017-10-01 01:25:30 | Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple |