On Wed, May 5, 2021 at 5:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I follow your concern, but I'm not convinced that this assertion is
> a useful aid; first because the asynchrony involved makes the edge
> cases rather squishy, and second because allowing 1024 bogus
> increments before complaining will likely mean that developer test
> runs will not last long enough to trigger the assertion, and third
> because if it does fire it's too far removed from the perpetrator
> to be much help in figuring out what went wrong, or even if
> anything *is* wrong.
Well, it allows 1024 bogus increments in one direction, but a negative
value will trip it PDQ.
> I've not tried to trace the code, but I'm now a bit suspicious
> that there is indeed a design bug here. I gather from the
> comments that parallel_register_count is incremented by the
> worker processes, which of course implies that a worker that
> fails to reattach to shared memory won't do that. But
> parallel_terminate_count is incremented by the postmaster.
> If the postmaster will do that even in the case of a worker that
> failed at startup, then lorikeet's symptoms are neatly explained.
parallel_register_count is incremented by
RegisterDynamicBackgroundWorker, i.e. when the worker process is
requested, not after it starts. To try to do it from the worker after
it's launched would, as you suppose, be completely busted. The more
general point here is that we should be adjusting these at the same
point we allocate and free the worker slots. Wherever that happens is
the right place to do this, because the goal of the mechanism is
precisely to limit the number of such slots that can be used by
parallel query.
--
Robert Haas
EDB: http://www.enterprisedb.com