Re: pg_background (and more parallelism infrastructure patches) - Mailing list pgsql-hackers

From Robert Haas
Subject Re: pg_background (and more parallelism infrastructure patches)
Date
Msg-id CA+TgmobdfKqdsxZYcpUf6J0ZkxgXGkdHT70eZCveWRQ29bppcQ@mail.gmail.com
Whole thread Raw
In response to Re: pg_background (and more parallelism infrastructure patches)  (Andres Freund <andres@2ndquadrant.com>)
Responses Re: pg_background (and more parallelism infrastructure patches)  (Andres Freund <andres@2ndquadrant.com>)
List pgsql-hackers
On Mon, Nov 10, 2014 at 4:58 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> No, that's not what I was thinking of. Consider:
>
> export pg_libdir=$(pg_config --libdir)
> mkdir $pg_libdir/plugins
> ln -s $pg_libdir/auto_explain.so $pg_libdir/plugins/
> PG_OPTIONS='-c local_preload_libraries=auto_explain' psql
>
> and use pg_background() (or something else using this infrastructure) in
> that context.
> Without having reread your relevant code I'd expect a:
> ERROR:  55P02: parameter "local_preload_libraries" cannot be set after connection start
> because the code would try to import the "user session's"
> local_preload_libraries values into the background process - after that
> actually already has done its initialization.

Thanks for these pointers.  Your directions turn out to be wrong in
detail, because it's PGOPTIONS not PG_OPTIONS, and the plugins
directory needs to be under $(pg_config --libdir)/postgresql, not just
$(pg_config --libdir).  But it was enough to get me on the right
track.  Long story short, it seems to work:

[rhaas ~]$ PGOPTIONS='-c local_preload_libraries=auto_explain' psql
psql (9.5devel)
Type "help" for help.

rhaas=# show local_preload_libraries;local_preload_libraries
-------------------------auto_explain
(1 row)

rhaas=# select * from pg_background_result(pg_background_launch('show
local_preload_libraries')) as (x text);     x
--------------auto_explain
(1 row)

This is what I expected, and I think what we want.  Background workers
enlisted for parallelism (or pg_background) need to be able to have
the same values for GUCs as the user backend that started them, and
our definition of setting things "at backend start" needs to be
expansive enough to include stuff we pull out of a dynamic shared
memory segment shortly thereafter.

>> It should end up with the same values there as the active session, not
>> the current one from postgresql.conf.  But we want to verify that's
>> the behavior we actually get.   Right?
>
> But that's also something worthwile to check.

Mmph.  There's a problem here.  I tried changing
local_preload_libraries='auto_explain' in postgresql.conf and then
sending PGC_SIGHUP.  A session started before that change does indeed
create a worker with that value still empty, but a session started
after that change also creates a worker with that value still empty.
Oops.  Not sure why that's happening yet, will investigate.

>> What would be even better is to find some way to MAKE IT SAFE to send
>> the undecoded tuple.  I'm not sure what that would look like.
>
> How do you define 'SAFE' here? Safe against concurrent SQL level
> activity? Safe against receiving arbitrary data?

The first one.  The code powering the background worker is just as
trustworthy as any other part of the backend, so it can be subverted
if you load malicious C code into the backend, but not otherwise.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Unintended restart after recovery error
Next
From: Andres Freund
Date:
Subject: Re: pg_background (and more parallelism infrastructure patches)