Re: [HACKERS] PoC: custom signal handler for extensions

From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: Maksim Milyutin <milyutinma(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] PoC: custom signal handler for extensions
Date: 2018-01-12 15:51:48
Message-ID: 73487e5b-38a6-c701-1be7-9f6ef7cb6cb8@sigaev.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> In perspective, this mechanism provides the low-level instrument to define
> remote procedure call on extension side. The simple RPC that defines effective
> userid on remote backend (remote_effective_user function) is attached for example.

Suppose, it's useful patch. Some notes:

1) AssignCustomProcSignalHandler is unneeded API function, easy replaced by
GetCustomProcSignalHandler/ReleaseCustomProcSignal/RegisterCustomProcSignalHandler
functions, but in other hand, it isn't looked as widely used feature to reassign
custom signal handler.

2) Naming RegisterCustomProcSignalHandler and ReleaseCustomProcSignal is not
self-consistent. Other parts suggest pair Register/Unregister or Aquire/Release.
Seems, Register/Unregister pair is preferred here.

3) Agree that Assert(reason >= PROCSIG_CUSTOM_1 && reason <= PROCSIG_CUSTOM_N)
should be replaced with ereport. By the way, ReleaseCustomProcSignal() is a
single place where there isn't a range check of reason arg

4) CustomSignalInterrupt() - play with errno and SetLatch() seems unneeded here
- there isn't call of handler of custom signal, SetLatch will be called several
lines below

5) Using a special memory context for handler call looks questionably, I think
that complicated handlers could manage memory itself (and will do, if they need
to store something between calls). So, I prefer to remove context.

6) As I can see, all possible (not only custom) signal handlers could perfectly
use this API, or, at least, be store in CustomHandlers array, which could be
renamed to InterruptHandlers, for example. Next, custom handler type is possible
to make closier to built-in handlers, let its signature extends to
void (*ProcSignalHandler_type) (ProcSignalReason reason) as others. It will
allow to use single handler to support several reasons.

7) Suppose, API allows to use different handlers in different processes for the
same reason, it's could be reason of confusion. I suggest to restrict
Register/Unregister call only for shared_preload_library, ie only during startup.

8) I'd like to see an example of usage this API somewhere in contrib in exsting
modules. Ideas?

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2018-01-12 16:01:53 Re: Possible performance regression with pg_dump of a large number of relations
Previous Message Peter Eisentraut 2018-01-12 15:49:53 Re: improve type conversion of SPI_processed in Python