Re: [PATCH] More docs on what to do and not do in extension code

Laurenz,

Thanks for your comments. Sorry it's taken me so long to get back to you.
Commenting inline below on anything I think needs comment; other proposed
changes look good.

On Sun, 30 May 2021 at 19:20, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> wrote:

> + always use <link
> linkend="xfunc-sleeping-interrupts-blocking">PostgreSQL's
> + interupt-aware APIs</link> for the purpose. Do not
> <function>usleep()</function>,
> + <function>system()</function>, make blocking system calls, etc.
> + </para>
>
> "system" is not a verb.
>

When it's a function call it is, but I'm fine with your revision:

Do not use <function>usleep()</function>, <function>system()</function>
> or other blocking system calls.
>
> + and should only use the main thread. PostgreSQL generally uses
> subprocesses
>
> Hm. If the extension does not start threads, it automatically uses the
> main thread.
> I think that should be removed for clarity.
>

IIRC I intended that to apply to the section that tries to say how to
survive running your own threads in the backend if you really must do so.

+ primitives like <function>WaitEventSetWait</function> where
> necessary. Any
> + potentially long-running loop should periodically call <function>
> + CHECK_FOR_INTERRUPTS()</function> to give PostgreSQL a chance to
> interrupt
> + the function in case of a shutdown request, query cancel, etc.
> This means
>
> Are there other causes than shutdown or query cancellation?
> At any rate, I am not fond of enumerations ending with "etc".
>

I guess. I wanted to emphasise that if you mess this up postgres might fail
to shut down or your backend might fail to respond to SIGTERM /
pg_terminate_backend, as those are the most commonly reported symptoms when
such issues are encountered.

+ by PostgreSQL if any function that it calls makes a
> + <function>CHECK_FOR_INTERRUPTS()</function> check. It may not
>
> "makes" sound kind of clumsy in my ears.
>

Yeah. I didn't come up with anything better right away but will look when I
get the chance to return to this patch.

> Attached is a new version that has my suggested changes, plus a few from
> Bharath Rupireddy (I do not agree with many of his suggestions).
>

Thanks very much. I will try to return to this soon and review the diff
then rebase and update the patch.

I have a large backlog to get through, and I've recently had the pleasure
of having to work on windows/powershell build system stuff, so it may still
take me a while.