Re: Plugins redux (was Re: [PATCHES] PL instrumentation plugin

From: "korryd(at)enterprisedb(dot)com" <korryd(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Plugins redux (was Re: [PATCHES] PL instrumentation plugin
Date: 2006-08-09 21:24:28
Message-ID: 1155158668.24313.64.camel@sakai.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

> ISTM there are two separate bits of functionality that the core backend
> needs to provide in support of PL plugins. The first is that we need a
> "rendezvous" facility whereby two separately-loaded shared libraries can
> connect and exchange data (a struct of function pointers for the immediate
> plans for plpgsql, but in general it might be anything). The exchanged
> data mustn't be limited to things known in advance to the core backend.
> The facility should support loading of communicating libraries in either
> order, and it should also allow for unloading of libraries. This seems
> easy enough to design: the core backend should basically provide a
> hashtable that maps names to "void *" pointer variables, and then two
> libraries that want to communicate just agree on a name and the data
> structure to be pointed to. (More details below.)

I like it so far...

> The other, probably more controversial bit of functionality is that there
> needs to be a way to cause a backend to load a PL plugin shared library
> without any cooperation from the connected client application. For
> interactive use of a PL debugger it might be sufficient to tell people to
> do "LOAD 'plpgsql_debugger'" before running their function-to-be-tested,
> but if you're trying to debug some behavior that only manifests in a large
> complicated application, it may be impractical to get the application to
> issue such a command.

Ok, but you should know that the PL/pgSQL debugger already handles this
without any cooperation from the client.

Loading the PL debugger means that your backend becomes debuggable, not
that it actually starts debugging.

If you want to debug a PL/pgSQL function invoked by some other process
(such as a Web server), you create a "global breakpoint". When a
debuggable process (that is, a process that has loaded the debugger
plugin) trips across the breakpoint, that's when it starts debugging.
(Note: a global breakpoint can identify a particular backend, by process
ID, or it can omit the process ID and trap the first debuggable backend
that trips across the breakpoint).

So, other than the (negligible) performance penalty, its safe to load
the debugger plugin into every backend. (If you've loaded the debugger,
the extra overhead is a single, lightweight-lock protected,
shared-memory hash lookup for each PL function invocation).

> We agreed while discussing this at the PG
> Anniversary Conference that providing a GUC variable for this purpose is
> probably sufficient, as there are multiple ways that such a variable can
> be set on behalf of an application: ALTER USER SET, ALTER DATABASE SET,
> "export PGOPTIONS=--var=val" before starting the app, etc. As submitted,
> the plugin patch envisions defining a custom GUC variable for each PL and
> having each PL add logic to force loading of any shared library mentioned
> in its specific variable. I claim however that that is just duplicative,
> and that a single backend-wide GUC variable should be sufficient.

Ok, so the rendevous variables replace the per-language GUC variables
(from my patch). In effect, a GUC variable is something that a user
would manage, a rendevous variable is something that two (or more)
pieces of code manage.

GUC variables are visible to the user, rendevous variables are visible
to dynamically loaded libraries.

Oh, and rendevous variables are insensitve to load order too.

Nice design.

> The principal difference between this proposal and the original patch
> is that this way supports unloading of PL plugins; AFAICS, the original
> patch would just crash if the plugin were unloaded and then use of plpgsql
> continued.

I don't think it could crash because there's no way to unload a plugin
(there is no UNLOAD statement, is there?).

Which reminds me, you haven't proposed a way to unload a shared-library.

> We need to think about security issues in defining such a variable:
> we don't want unprivileged users to be able to load arbitrary code
> into the server. One solution to this is to make the variable SUSET,
> ie, you must be superuser to set it. This would make it impossible
> for people to debug their own functions without superuser privileges
> ... but IIRC actual use of the proposed plpgsql debugger requires
> superuser privileges too, so this isn't necessarily a fatal objection.

Correct (the debugger currently requires superuser privileges). We'll
probably want to relax that later.

> Still, it'd be nice if the PGOPTIONS way of setting the variable could
> be used without requiring that the client app itself run as superuser.
> Plan B is to restrict which shared libraries can be loaded using the
> variable --- there are different ways we could do that, but my suggestion
> would be to insist that the shared library come from "$libdir/plugins/".
> The DBA is then responsible for allowing only "safe" shared libraries
> to be installed in that directory, and therefore we can allow unprivileged
> users to set "backend_load_libraries". (We might as well also allow them
> to execute LOAD for these libraries, so that plugins could be switched
> intra-session without superuser privileges.)

How about a combination of plan A and plan B? Make
backend_load_libraries a USERSET variable, but you can't *add* libraries
outside of $libdir/plugins/ unless you are a superuser.

> BTW, is anyone up for renaming the existing "preload_libraries" variable
> to "postmaster_load_libraries"? This would be more symmetrical with
> "backend_load_libraries", and so perhaps clearer about which does what

Makes sense to me, of course that breaks existing postgresql.conf files.

Do you want me to do any of this coding?

--
Korry Douglas korryd(at)enterprisedb(dot)com
EnterpriseDB http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Brian C. DeRocher 2006-08-09 21:24:41 numerics lose scale and precision in views of unions
Previous Message Bruce Momjian 2006-08-09 21:19:10 Re: WIN32 Build?

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2006-08-09 22:04:49 Re: Plugins redux (was Re: [PATCHES] PL instrumentation plugin
Previous Message Bruce Momjian 2006-08-09 21:18:22 Re: [HACKERS] [PATCHES] BUG #2569: statement_timeout bug on