Re: Comment patch for bgworker.c

On Fri, Oct 24, 2014 at 8:51 PM, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:
> The comment for the BackgroundWorkerSlot structure tripped me up reviewing
> Robert's background worker patch; it made it clear that you need to use a
> memory barrier before setting in_use, but normally you'd never need to worry
> about that because RegisterDynamicBackgroundWorker() handles it for you.
> Patch adds a comment to that effect.

I vote to reject this patch. I think it's explaining something that
doesn't really need to be explained, and shouldn't be explained like
this even if it does. It adds a comment that reads "Note that
RegisterDynamicBackgroundWorker() handles in_use correctly for you".
But the long block comment of which it is a part is entirely devoted
to explaining concerns internal to bgworker.c, from which I think it
should be inferred that all of the public APIs in that file handle all
of the things in that paragraph correctly (or are intended to,
anyway).

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