From: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Subject: Re: pgsql: Add test for postmaster crash restarts.
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 pgsql-committers <pgsql-committers@postgresql.org>
References: <17084.1505837634@sss.pgh.pa.us>
 <20170919164718.66hcedq2rtlkntvf@alap3.anarazel.de>
 <18695.1505840008@sss.pgh.pa.us>
 <D2FBABBF-F0E3-43FE-8EA4-E2431DD6EEBA@anarazel.de>
 <20170919174232.soc2skrqxxdtnh22@alap3.anarazel.de>
 <21486.1506601297@sss.pgh.pa.us>
 <5ce9b1f4-dff8-6fe9-74ab-d8e1b993b1d5@2ndQuadrant.com>
 <20170930221043.iay4s7rmwm5l7wiw@alap3.anarazel.de>
 <56b65da3-f7cc-494c-7952-ba5f6c804d7e@2ndQuadrant.com>
 <20170930222712.xgdgxnt4sjvig5mn@alap3.anarazel.de>
 <20170930224424.ud5ilchmclbl5y5n@alap3.anarazel.de>
Message-ID: <75777957-d16f-f799-d0ec-bc3d5b7c72de@2ndQuadrant.com>
Date: Sat, 30 Sep 2017 22:28:39 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <20170930224424.ud5ilchmclbl5y5n@alap3.anarazel.de>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Content-Language: en-MW
Precedence: bulk
Sender: pgsql-committers-owner@postgresql.org


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


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers