Thread: Re: [PATCHES] 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

<blockquote type="CITE"><pre>
<font color="#000000">Applied with a few small changes --- I renamed the GUC variables after a</font>
<font color="#000000">suggestion by Simon Riggs, and fixed things so that</font>
<font color="#000000">backend_load_libraries could actually do something useful (you had it as</font>
<font color="#000000">PGC_POSTMASTER, making it effectively no more flexible than the existing</font>
<font color="#000000">preload_libraries list).</font>
</pre></blockquote><br /> 'Doh! That was a cut/paste error on my part.  Thanks.<br /><br /><blockquote
type="CITE"><pre>
<font color="#000000">The only change that will directly impact your code is that I thought</font>
<font color="#000000">it'd be better to provide plpgsql_exec_error_callback and</font>
<font color="#000000">exec_assign_expr as separate fields instead of arguments to func_setup,</font>
<font color="#000000">viz</font>

<font color="#000000">    if (*plugin_ptr)</font>
<font color="#000000">    {</font>
<font color="#000000">        (*plugin_ptr)->error_callback = plpgsql_exec_error_callback;</font>
<font color="#000000">        (*plugin_ptr)->assign_expr = exec_assign_expr;</font>

<font color="#000000">        if ((*plugin_ptr)->func_setup)</font>
<font color="#000000">            ((*plugin_ptr)->func_setup)(estate, func);</font>
<font color="#000000">    }</font>

<font color="#000000">I'm not totally wedded to this if you don't like it, but my thought was</font>
<font color="#000000">that passing these as arguments to func_setup would mean a lot of pain</font>
<font color="#000000">anytime we wanted to change the set of function pointers provided:</font>
<font color="#000000">every plugin would need textual changes whether it actually used these</font>
<font color="#000000">functions or not.</font>
</pre></blockquote><br /> Good idea, those two function pointers are sort of necessary-evil required only by the
debuggerplugin (other plugins presumably won't need them).<br /><br /><blockquote type="CITE"><pre>
 
<font color="#000000">> I have not implemented any support for unloading shared libraries.  Once</font>
<font color="#000000">> we've finalized the design for rendezvous variables, I'll submit a</font>
<font color="#000000">> separate documentation patch.</font>

<font color="#000000">I added docs for the GUC variables but didn't do more than that.  I</font>
<font color="#000000">think the code comments are probably sufficient as far as rendezvous</font>
<font color="#000000">variables and PLpgSQL_plugin go ... did you have something else in mind?</font>
</pre></blockquote><br /> I'll look around the existing documentation to see if I can find an appropriate place, but we
don'treally have an implementor's guide.<br /><br /> Thanks for your help and suggestions.<br /><br />             --
Korry<br/><table cellpadding="0" cellspacing="0" width="100%"><tr><td><br /><br /> --<br />   Korry Douglas    <a
href="mailto:korryd@enterprisedb.com">korryd@enterprisedb.com</a><br/>   EnterpriseDB      <a
href="http://www.enterprisedb.com">http://www.enterprisedb.com</a></td></tr></table>