From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | jcasanov(at)systemguards(dot)com(dot)ec, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Review: support for multiplexing SIGUSR1 |
Date: | 2009-07-27 07:13:27 |
Message-ID: | 3f0b79eb0907270013l39b2e85bjdb3ad96c6394e2e4@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
Thanks for reviewing the patch!
On Mon, Jul 27, 2009 at 2:43 AM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> 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.
OK, I'll change the mechanism to assign a ProcSignalSlot to a process,
independently from ProcState, but similarly to ProcState: a process gets
a ProcSignalSlot from newly-introduced freelist at startup, and returns it
to freelist at exit. Since this assignment and return require the lock, I'll
introduce a new LWLock for them.
> 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?
Yes, I'm planning to use this mechanism for communication between
a process which can generate the WAL records and a WAL sender process,
in Synch Rep. Since not only a backend but also some auxiliary processes
(e.g., bgwriter) can generate the WAL records, processes which don't have
a ProcState need to receive the multiplexed signal, too.
> 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.
This makes sense. I'll get rid of a spinlock from the patch and add the usage
note as above.
> 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?
In, at least, currently-intended use case (i.e., Synch Rep), such wrong signal
is not fatal because it's only used as a hint.
> 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.
Umm... the patch should cover a notify interrupt which currently uses
SIGUSR2?
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2009-07-27 07:56:06 | Re: Non-blocking communication between a frontend and a backend (pqcomm) |
Previous Message | Magnus Hagander | 2009-07-27 07:11:32 | Re: Multicore builds on MSVC |