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 |
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 |