Re: logical replication launcher crash on buildfarm - Mailing list pgsql-hackers

From Petr Jelinek
Subject Re: logical replication launcher crash on buildfarm
Date
Msg-id 2124fb62-2393-8525-4fe9-b95b1b689d09@2ndquadrant.com
Whole thread Raw
In response to Re: logical replication launcher crash on buildfarm  (Petr Jelinek <petr.jelinek@2ndquadrant.com>)
Responses Re: logical replication launcher crash on buildfarm  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On 29/03/17 01:29, Petr Jelinek wrote:
> On 28/03/17 18:05, Petr Jelinek wrote:
>> On 28/03/17 17:55, Robert Haas wrote:
>>> On Mon, Mar 27, 2017 at 11:20 PM, Petr Jelinek
>>> <petr.jelinek@2ndquadrant.com> wrote:
>>>> On 28/03/17 04:46, Robert Haas wrote:
>>>>> On Mon, Mar 27, 2017 at 10:04 PM, Andres Freund <andres@anarazel.de> wrote:
>>>>>>> Btw now that I look at the code, I guess we'll want to get rid of
>>>>>>> bgw_main completely in HEAD given that we can't guarantee it will be
>>>>>>> valid even for shared_preload_library libraries. For older branches I
>>>>>>> would leave things as they are in this regard as there don't seem to be
>>>>>>> any immediate issue for standard binaries.
>>>>>>
>>>>>> As long as you fix it so culicidae is happy (in 9.6) ;).  I think it's
>>>>>> fine to just introduce bgw_builtin_id or such, and leave the bgw_main
>>>>>> code in place in < HEAD.
>>>>>
>>>>> I wasn't thinking of introducing bgw_builtin_id.  My idea was just
>>>>> along the lines of
>>>>>
>>>>> if (bgw_library_name == NULL && bgw_function_name != NULL)
>>>>> {
>>>>>     if (strcmp(bgw_function_name, "ParallelQueryMain") == 0)
>>>>>        ParallelQueryMain(blah);
>>>>>     else if (strcmp(bgw_function_name, "LogicalReplicationMain") == 0)
>>>>>        LogicalReplicationMain(blah);
>>>>> }
>>>>>
>>>>> I think something like that is certainly better for the back-branches,
>>>>> because it doesn't cause an ABI break.  But I think it would also be
>>>>> fine for master.
>>>>>
>>>>
>>>> I had something slightly more complex like the attached in mind.
>>>
>>> Seems broadly reasonable on a quick look, but I think we should leave
>>> bgw_main intact in 9.6.  It may be working for fine for people who
>>> don't care about Windows, and I'd rather not break it gratuitously.
>>> Can we have two patches, one of which converts the internal stuff to
>>> use the new mechanism and a second of which removes bgw_main?  The
>>> second one, at least, also needs to update the documentation.  (A good
>>> practice when removing an identifier is to run 'git grep
>>> thing_i_am_removing' after removing it...)
>>>
>>
>> Yes I agree (and I said it in the thread) I plan to submit the 9.6 too,
>> I just sent what I had before having it all ready mainly because you
>> started talking about code already and I didn't want you to waste time
>> with it needlessly, but it was already 5AM for me ;).
>>
> 
> Okay finally got to it, the 9.6 adds similar mechanism as HEAD (it could
> be simpler there but it seems like same code is better) but does not
> remove the bgw_main.
> 

Sigh, forgot git add for the docs, so one more try...

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

pgsql-hackers by date:

Previous
From: Petr Jelinek
Date:
Subject: Re: logical replication launcher crash on buildfarm
Next
From: Craig Ringer
Date:
Subject: Re: Logical decoding on standby