Re: Review: support for multiplexing SIGUSR1

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: jcasanov(at)systemguards(dot)com(dot)ec, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Review: support for multiplexing SIGUSR1
Date: 2009-07-26 17:43:19
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> I updated the patch to solve two problems which you pointed.

> Here is the changes:

> * Prevented the obsolete flag to being set to a new process, by using
> newly-introduced spinlock.

> * Used the index of AuxiliaryProcs instead of auxType, to assign
> backend ID to an auxiliary process.

Neither of these changes seem like a good idea to me. The use of a
spinlock renders the mechanism unsafe for use from the postmaster ---
we do not wish the postmaster to risk getting stuck if the contents of
shared memory have become corrupted, eg, there is a spinlock that looks
taken. And you've completely mangled the concept of BackendId.
MyBackendId is by definition the same as the index of the process's
entry in the sinval ProcState array. This means that (1) storing it in
the ProcState entry is silly, and (2) assigning values that don't
correspond to an actual ProcState entry is dangerous.

If we want to be able to signal processes that don't have a ProcState
entry, it would be better to assign an independent index instead of
overloading BackendId like this. I'm not sure though whether we care
about being able to signal such processes. It's certainly not necessary
for catchup interrupts, but it might be for future applications of the
mechanism. Do we have a clear idea of what the future applications are?

As for the locking issue, I'm inclined to just specify that uses of the
mechanism must be such that receiving a signal that wasn't meant for you
isn't fatal. In the case of catchup interrupts the worst that can
happen is you do a little bit of useless work. Are there any intended
uses where it would be seriously bad to get a misdirected signal?

I agree with Jaime that the patch would be more credible if it covered
more than one signal usage at the start --- these questions make it
clear that the design can't happen in a vacuum without intended usages
in mind.

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2009-07-26 17:49:32 Re: When is a record NULL?
Previous Message Greg Stark 2009-07-26 17:40:29 Re: autogenerating headers & bki stuff