Re: logical replication launcher crash on buildfarm

Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> writes:
> So this is what I came up with on plane. Generalized the loading
> functionality into load_library_function which that can load either
> known postgres functions or call load_external_function.

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.

> I am not quite sure if fmgr.c is best place to put it, but I didn't want
> to include stuff from executor in bgworker.c.

I really did not care for putting those references into fmgr.c. Aside
from the layering violations involved, there was a type cheat, in that
you had functions with entirely different signatures in the same list.
I eventually decided that the least ugly solution was to create two
different pointer arrays and frontend functions, one for bgworkers and
one for parallel workers. That ends up requiring only one extra
export/import, to make ParallelQueryMain accessible in parallel.c.
Maybe we need to rethink the division of labor between parallel.c
and execParallel.c, but that would depend on somebody explaining
what the difference is in the first place.

> As with previous patch, 9.6 will need quite different patch as we need
> to keep compatibility there,

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.
The only thing that actually works is to call the undocumented
CreateParallelContextForExternalFunction. I suggest that we could
back-patch this as-is, if we add CreateParallelContextForExternalFunction
as a trivial wrapper around CreateParallelContext.

regards, tom lane