Re: logical replication launcher crash on buildfarm

Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
> On 15/04/17 06:01, Tom Lane wrote:
>> I've had more than enough of seeing buildfarm failures from culicidae,
>> so I whacked this around until I was happy with it and pushed it.
>> Further adjustments welcome of course.

> Thanks. Seems like culicidae is finally happy with master.

Well, only sort of happy, but I see how we can fix that.

>> I don't really understand why 9.6 needs a significantly different
>> patch.  AFAICS, it simply does not work (with any reliability)
>> for a loadable module to call CreateParallelContext directly in 9.6.

> I think the problem is that if somebody was using CreateParallelContext
> it may have worked on unix when just normally forking if the extension
> was loaded via shared_preload_libraries. And lot of extensions are linux
> only. So we might break working setup for somebody if we change the
> function signature.

Ah, I didn't think of shared_preload_libraries.  OK, we can't change
the signature in released branches.

> We may need to keep CreateParallelContext as is in back branches and add
> some CreateParallelContextInternal which would do what
> CreateParallelContext does in master (and is used by postgres) and then
> make CreateParallelContextForExternalFunction simple wrapper around
> that. It's somewhat ugly though.

Yeah, but ugly compatibility hacks in back branches are pretty common.
But rather than inventing "CreateParallelContextInternal", I suggest
we just have the core code call CreateParallelContextForExternalFunction.

			regards, tom lane