On Wed, 21 Jun 2017 12:06:33 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:
> On Tue, Jun 20, 2017 at 3:24 AM, Daniel Gustafsson <daniel@yesql.se> wrote:
> > The message is stored in a new shmem area which is checked when the session is
> > aborted. To keep things simple a small buffer is kept per backend for the
> > message. If deemed too costly, keeping a central buffer from which slabs are
> > allocated can be done (but seemed rather complicated for little gain compared
> > to the quite moderate memory spend.)
>
> I think that you are right to take the approach with a per-backend
> slot. This will avoid complications related to entry removals and
> locking issues. There would be scaling issues as well if things get
> very signaled for a lot of backends.
>
> +#define MAX_CANCEL_MSG 128
> That looks enough.
>
> + LWLockAcquire(BackendCancelLock, LW_EXCLUSIVE);
> +
> + strlcpy(slot->message, message, sizeof(slot->message));
> + slot->len = strlen(message);
> Why not using one spin lock per slot and save it in BackendCancelShmemStruct?
+1
I found an example that a spin lock is used during strlcpy in WalReceiverMain().
>
> + pid_t pid = PG_GETARG_INT32(0);
> + char *msg = text_to_cstring(PG_GETARG_TEXT_PP(1));
> +
> + PG_RETURN_BOOL(pg_terminate_backend_internal(pid, msg));
> It would be more solid to add some error handling for messages that
> are too long, or at least truncate the message if too long.
I agree that error handling for too long messages is needed.
Although long messages are truncated in SetBackendCancelMessage(),
it is better to inform users that the message they can read was
truncated one. Or, maybe we should prohibit too long message
is passed in pg_teminate_backend()
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
--
Yugo Nagata <nagata@sraoss.co.jp>