Re: logical replication launcher crash on buildfarm

From: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>
Subject: Re: logical replication launcher crash on buildfarm
Date: 2017-03-28 16:05:35
Message-ID: 0471e978-b121-ad2f-2879-f1694d9a5ce7@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 ;).

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-03-28 16:18:32 Re: Removing binaries
Previous Message Mark Dilger 2017-03-28 16:04:15 Re: Monitoring roles patch