Re: logical replication launcher crash on buildfarm

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(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>>> On 28/03/17 04:46, Robert Haas wrote:
>>>> On Mon, Mar 27, 2017 at 10:04 PM, Andres Freund <andres(at)anarazel(dot)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.

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