Re: pg_background (and more parallelism infrastructure patches)

On Wed, Nov 12, 2014 at 11:36 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Nov 12, 2014 at 11:19 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> The question is whether the library is actually loaded in that case?
>> Because that normally only happens early during startup - which is why
>> it's a PGC_BACKEND guc.
>
> It looks like that does not work.
>
> [rhaas pgsql]$ PGOPTIONS='-c local_preload_libraries=auto_explain' psql
> psql (9.5devel)
> Type "help" for help.
>
> rhaas=# select * from pg_background_result(pg_background_launch('show
> auto_explain.log_min_duration')) as (x text);
> ERROR:  unrecognized configuration parameter "auto_explain.log_min_duration"
> CONTEXT:  background worker, pid 31316
>
> So, there's more to be done here.  Rats.

It turned out to be quite simple to fix both problems.  This
particular case fails because the call that loads the libraries
specified by session_preload_libraries and local_preload_libraries is
in PostgresMain() and thus never gets called by pg_background.  I
fixed that by adding that call to pg_background in the appropriate
place.  While I was at it, I added the REVOKE statements we discussed
earlier to pg_background's .sql file.

The other problem was due to this code in set_config_option:

                                /*
                                 * If a PGC_BACKEND or PGC_SU_BACKEND
parameter is changed in
                                 * the config file, we want to accept
the new value in the
                                 * postmaster (whence it will propagate to
                                 * subsequently-started backends), but
ignore it in existing
                                 * backends.  This is a tad klugy, but
necessary because we
                                 * don't re-read the config file
during backend start.
                                 *
                                 * In EXEC_BACKEND builds, this works
differently: we load all
                                 * nondefault settings from the
CONFIG_EXEC_PARAMS file during
                                 * backend start.  In that case we
must accept PGC_SIGHUP
                                 * settings, so as to have the same
value as if we'd forked
                                 * from the postmaster.  We detect
this situation by checking
                                 * IsInitProcessingMode, which is a
bit ugly, but it doesn't
                                 * seem worth passing down an explicit
flag saying we're doing
                                 * read_nondefault_variables().
                                 */
#ifdef EXEC_BACKEND
                                if (IsUnderPostmaster &&
!IsInitProcessingMode())
                                        return -1;
#else
                                if (IsUnderPostmaster)
                                        return -1;
#endif

When restoring variables via RestoreGUCState(), we need the same kind
of special-handling that we do when running in EXEC_BACKEND mode and
restoring variables via read_nondefault_variables().  Extending the
IsInitProcessingMode kludge() doesn't look appealing, so I instead
added the flag contemplated by the comment.

Updated patches attached.

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