PL instrumentation plugin and Rendezvous variable support - version 2

"korryd@enterprisedb.com" <korryd@enterprisedb.com> writes:

This is an updated version of the PL instrumentation plugin patch that I
submitted on July-28. The new version re-implements the plugin loader
code to use "rendezvous variables" as suggested by Tom Lane (thanks Tom,
very elegant design).

Applied with a few small changes --- I renamed the GUC variables after a
suggestion by Simon Riggs, and fixed things so that
backend_load_libraries could actually do something useful (you had it as
PGC_POSTMASTER, making it effectively no more flexible than the existing
preload_libraries list).

The only change that will directly impact your code is that I thought
it'd be better to provide plpgsql_exec_error_callback and
exec_assign_expr as separate fields instead of arguments to func_setup,
viz

if (*plugin_ptr)
{
(*plugin_ptr)->error_callback = plpgsql_exec_error_callback;
(*plugin_ptr)->assign_expr = exec_assign_expr;

if ((*plugin_ptr)->func_setup)
((*plugin_ptr)->func_setup)(estate, func);
}

I'm not totally wedded to this if you don't like it, but my thought was
that passing these as arguments to func_setup would mean a lot of pain
anytime we wanted to change the set of function pointers provided:
every plugin would need textual changes whether it actually used these
functions or not.

I have not implemented any support for unloading shared libraries. Once
we've finalized the design for rendezvous variables, I'll submit a
separate documentation patch.

I added docs for the GUC variables but didn't do more than that. I
think the code comments are probably sufficient as far as rendezvous
variables and PLpgSQL_plugin go ... did you have something else in mind?

regards, tom lane

Applied with a few small changes --- I renamed the GUC variables after a
suggestion by Simon Riggs, and fixed things so that
backend_load_libraries could actually do something useful (you had it as
PGC_POSTMASTER, making it effectively no more flexible than the existing
preload_libraries list).

'Doh! That was a cut/paste error on my part. Thanks.

The only change that will directly impact your code is that I thought
it'd be better to provide plpgsql_exec_error_callback and
exec_assign_expr as separate fields instead of arguments to func_setup,
viz

if (*plugin_ptr)
{
(*plugin_ptr)->error_callback = plpgsql_exec_error_callback;
(*plugin_ptr)->assign_expr = exec_assign_expr;

if ((*plugin_ptr)->func_setup)
((*plugin_ptr)->func_setup)(estate, func);
}

I'm not totally wedded to this if you don't like it, but my thought was
that passing these as arguments to func_setup would mean a lot of pain
anytime we wanted to change the set of function pointers provided:
every plugin would need textual changes whether it actually used these
functions or not.

Good idea, those two function pointers are sort of necessary-evil
required only by the debugger plugin (other plugins presumably won't
need them).

I have not implemented any support for unloading shared libraries. Once
we've finalized the design for rendezvous variables, I'll submit a
separate documentation patch.

I added docs for the GUC variables but didn't do more than that. I
think the code comments are probably sufficient as far as rendezvous
variables and PLpgSQL_plugin go ... did you have something else in mind?

I'll look around the existing documentation to see if I can find an
appropriate place, but we don't really have an implementor's guide.

Thanks for your help and suggestions.

-- Korry

--
Korry Douglas korryd@enterprisedb.com
EnterpriseDB http://www.enterprisedb.com