Re: bgworker / SPI docs patches

On Wed, Jul 29, 2015 at 11:35 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Feb 17, 2015 at 3:29 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
>> After spending a while working on the BDR extension I've found that the
>> current documentation on the SPI and bgworkers could use some more detail.
>>
>> In the process I've put cross reference labels in for most chapter headings,
>> as I got sick of seeing "Chapter 33" (or whatever) everywhere in the docs.
>> I'd like to progressively add them to every section, refentry, chapter, etc
>> that has an id.
>>
>> I hope this is useful.
>
> Per Peter, 0001 is not wanted.  I tend to agree with him on that point
> of style.  Patch 0002 makes the docs build fail unless 0001 is applied
> first, so I can't pursue that unless you want to revise that
> accordingly.  I've committed 0003, and will look at 0004 next.

+   <warning>
+    <para>
+     Use of this field is deprecated. It should be set to
+     <literal>NULL</literal> then <structfield>bgw_library_name</structfield>
+     and <structfield>bgw_function_name</structfield> should be used instead.
+    </para>

I don't think bgw_main is exactly deprecated.  It's fine to use it if
the function is in the core code; it just can't be safely used for
functions in dynamically loaded shared libraries.  Maybe that's close
enough to "deprecated" that we should just call it deprecated, but I'm
slightly reluctant to use that word.

   <para>
    <structfield>bgw_notify_pid</structfield> is the PID of a PostgreSQL
    backend process to which the postmaster should send <literal>SIGUSR1</>
-   when the process is started or exits.  It should be 0 for workers registered
-   at postmaster startup time, or when the backend registering the worker does
+   when the process is started or exits, causing the notified process's latch
+   to be set.  It should be 0 for workers registered
+   at postmaster startup time, when the backend registering the worker does
    not wish to wait for the worker to start up.  Otherwise, it should be
    initialized to <literal>MyProcPid</>.
   </para>

This isn't actually right.  You can use set_latch_on_sigusr1 to get
that behavior, but it's not the default behavior.

    <varname>dboid</> is <literal>InvalidOid</>, the session is not connected
-   to any particular database, but shared catalogs can be accessed.
+   to any particular database, but shared catalogs (tables where
+   <varname>relisshared</varname> is <literal>true</literal> in
+   <varname>pg_class</varname>) can be accessed.
    If <varname>username</> is NULL or <varname>useroid</> is

I don't think we should be defining the term "shared catalogs" in the
section on background workers.  If it needs to be explained, let's do
that elsewhere (or find where it's already done) and link to it.

    A background worker can only call one of these two functions, and only
-   once.  It is not possible to switch databases.
+   once.  It is not possible to switch databases without exiting and restarting
+   the background worker.

That's not really switching databases, though, is it?

I've committed some bits of this that seem useful and controversial
with rather extensive wordsmithing; let me know if it doesn't look
good.

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