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.a...@cybertec.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.
