Dependency between bgw_notify_pid and bgw_flags

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Dependency between bgw_notify_pid and bgw_flags
Date: 2015-06-04 12:52:19
Message-ID: CAFjFpRfwS-xCcTnou5TLjNuayWB6g5RJPkkv0dSKRSf1WXeHTg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,
Documentation here http://www.postgresql.org/docs/devel/static/bgworker.html
does not indicate any relation between the fields bgw_notify_pid and
bgw_flags of BackgroundWorker structure. But in one has to set
BGWORKER_BACKEND_DATABASE_CONNECTION in order to use bgw_notify_pid feature.

In BackgroundWorkerStateChange
318 /*
319 * Copy the PID to be notified about state changes, but only
if the
320 * postmaster knows about a backend with that PID. It isn't
an error
321 * if the postmaster doesn't know about the PID, because the
backend
322 * that requested the worker could have died (or been killed)
just
323 * after doing so. Nonetheless, at least until we get some
experience
324 * with how this plays out in the wild, log a message at a
relative
325 * high debug level.
326 */
327 rw->rw_worker.bgw_notify_pid = slot->worker.bgw_notify_pid;
328 if
(!PostmasterMarkPIDForWorkerNotify(rw->rw_worker.bgw_notify_pid))
329 {
330 elog(DEBUG1, "worker notification PID %lu is not valid",
331 (long) rw->rw_worker.bgw_notify_pid);
332 rw->rw_worker.bgw_notify_pid = 0;
333 }

bgw_notify_pid gets wiped out (and that too silently) if
PostmasterMarkPIDForWorkerNotify() returns false.
PostmasterMarkPIDForWorkerNotify() only looks at the BackendList, which
does not contain all the background worker created by
Register*BackgroundWorker() calls. Only a baground worker which has set
BGWORKER_BACKEND_DATABASE_CONNECTION, gets added into BackendList in
maybe_start_bgworker()

5629 if (rw->rw_worker.bgw_flags &
BGWORKER_BACKEND_DATABASE_CONNECTION)
5630 {
5631 if (!assign_backendlist_entry(rw))
5632 return;
5633 }
5634 else
5635 rw->rw_child_slot = MyPMChildSlot =
AssignPostmasterChildSlot();
5636
5637 do_start_bgworker(rw); /* sets rw->rw_pid */
5638
5639 if (rw->rw_backend)
5640 {
5641 dlist_push_head(&BackendList, &rw->rw_backend->elem);

Should we change the documentation to say "one needs to set
BGWORKER_BACKEND_DATABASE_CONNECTION" in order to use bgw_notify_pid
feature? OR we should fix the code not to wipe out bgw_notify_pid in the
code above (may be change PostmasterMarkPIDForWorkerNotify() to scan the
list of other background workers as well)?
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2015-06-04 13:23:56 Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file
Previous Message Fujii Masao 2015-06-04 10:55:53 Re: Missing "-i / --ignore-version" in pg_dump help