Re: Multiplexing SUGUSR1

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Multiplexing SUGUSR1
Date: 2008-12-10 10:45:02
Message-ID: 493F9DAE.5090305@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Fujii Masao wrote:
> Hi,
>
> On Wed, Dec 10, 2008 at 1:43 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>>> I'm surprised you feel that way. You suggested earlier
>>> (http://archives.postgresql.org/message-id/28487.1221147665@sss.pgh.pa.us)
>>> that a solution that only works for processes attached to shared memory
>>> would probably suffice for now.
>> Well, I wasn't complaining about the dependence on being attached to
>> shared memory. What I'm complaining about is the dependence on the
>> rather complex PGPROC data structure.
>>
>>> That seems hard, considering that we also want it to work without
>>> locking. Hmm, I presume we can use spinlocks in a signal handler?
>>> Perhaps some sort of a hash table protected by a spinlock would work.
>> No, locks are right out if the postmaster is supposed to be able to use
>> it. What I was thinking of is a simple linear array of PIDs and
>> sig_atomic_t flags. The slots could be assigned on the basis of
>> backendid, but callers trying to send a signal would have to scan the
>> array looking for the matching PID. (This doesn't seem outlandishly
>> expensive considering that one is about to do a kernel call anyway.
>> You might be able to save a few cycles by having the PID array separate
>> from the flag array, which should improve the cache friendliness of the
>> scan.) Also, for those callers who do have access to a PGPROC, there
>> could be a separate entry point that passes backendid instead of PID to
>> eliminate the search.
>
> Thanks for the comment!
> I updated the patch so. Is this patch ready to apply?

Looks like we stepped on each others toes, I was just about to send a
similar patch. Attached is my version, it's essentially the same as yours.

My version doesn't have support for auxiliary processes. Does the
synchronous replication patch need that?

This comment is wrong, though the code below it is right:

> *** base/src/backend/bootstrap/bootstrap.c 2008-12-10 16:29:10.000000000 +0900
> --- new/src/backend/bootstrap/bootstrap.c 2008-12-10 17:16:23.000000000 +0900
> ***************
> *** 389,394 ****
> --- 389,403 ----
> InitAuxiliaryProcess();
> #endif
>
> + /*
> + * Assign backend ID to auxiliary processes like backends, in order to
> + * allow multiplexing signal to auxiliary processes. Since backends use
> + * ID in the range from 1 to MaxBackends, we assign auxiliary processes
> + * with MaxBackends + AuxProcType as an unique ID.
> + */
> + MyBackendId = MaxBackends + auxType;
> + MyProc->backendId = MyBackendId;
> +
> /* finish setting up bufmgr.c */
> InitBufferPoolBackend();

Backends use IDs in the range 0 to MaxBackends-1, inclusive.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Attachment Content-Type Size
signal_handling-heikki-2.patch text/x-diff 17.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2008-12-10 10:49:45 Re: Multiplexing SUGUSR1
Previous Message Fujii Masao 2008-12-10 10:29:58 Re: Multiplexing SUGUSR1