Обсуждение: Additional message in pg_terminate_backend

Поиск
Список
Период
Сортировка

Additional message in pg_terminate_backend

От
Roman Khapov
Дата:
Hi hackers!

Recently I started working on patch for adding additional message from admin
in pg_terminate_backend for one of our greenplum fork. The main idea is that there must be
done little investigation every time you see 'FATAL:  terminating connection due to administrator command’ to
understandthe reason, especially in cases where connection was terminated by another 
user. So it was decided to create some new functions, that allows to terminate connection with
additional message.

I did POC patches with the next main ideas:
 - lets add termReasonStr field in every PGPROC, that field can be used in ProcessInterrupts()
 - implementation of pg_terminate_backend/pg_cancel_backend should be accessible from extensions, so lets move it in
pg_terminate_backend_impl/pg_cancel_backend_impland add definitions for it somewhere 
 - write simple extensions, which defines functions like pg_terminate_backend_msg, that sets termReasonStr and calls
pg_terminate_backend_impl

After patch and added extension, it is possible to do smth like:
postgres=# select pg_terminate_backend_msg(pg_backend_pid(), 0, ’the message');
FATAL:  terminating connection due to administrator command: the message

The general question I want to ask: can this patches be useful for vanilla PostgreSQL?

If so, there are some questions about patch improvements:
 - maybe the message can be delivered to backend by some other way than the field in struct PGPROC?
termReasonStr field consumes some shared memory and are used in rare cases
 - names of all new funcs/fields/etc should be changed to some better names
 - new file signalfuncs.h seems like too complicated solutions to define *_impl functions, it can be done in
some other files?

-----
Best regards,
Roman Khapov


Вложения

Re: Additional message in pg_terminate_backend

От
Andrey Borodin
Дата:

> On 13 Dec 2025, at 12:44, Roman Khapov <rkhapov@yandex-team.ru> wrote:
>
> Recently I started working on patch for adding additional message from admin
> in pg_terminate_backend for one of our greenplum fork. The main idea is that there must be
> done little investigation every time you see 'FATAL:  terminating connection due to administrator command’ to
understandthe reason, especially in cases where connection was terminated by another 
> user. So it was decided to create some new functions, that allows to terminate connection with
> additional message.

Overall idea seems good to me.
Keep in mind that Postgres literals are translated into many languages. So text ought to be clear enough for
translatorsto build a sentence that precedes termination reason. 

>
> I did POC patches with the next main ideas:
> - lets add termReasonStr field in every PGPROC, that field can be used in ProcessInterrupts()
> - implementation of pg_terminate_backend/pg_cancel_backend should be accessible from extensions, so lets move it in
pg_terminate_backend_impl/pg_cancel_backend_impland add definitions for it somewhere 
> - write simple extensions, which defines functions like pg_terminate_backend_msg, that sets termReasonStr and calls
pg_terminate_backend_impl

First thing that we need to do is to agree on API of the new feature. We do not need core-extension separation for
this.
My vote would be for having pg_cancel_backend(reason text)\pg_terminate_backend(reason text) along with parameterless
versions.
32 bytes per PGPROC seems reasonable for a "reason". The patch doesn't seem to take care of cleaning "termReasonStr".
Isit done elsewhere? 
We have a race condition if many backends cancel same backend. Won't they mess each other's reason?

Thanks!


Best regards, Andrey Borodin.




Re: Additional message in pg_terminate_backend

От
Daniel Gustafsson
Дата:
> On 13 Dec 2025, at 08:44, Roman Khapov <rkhapov@yandex-team.ru> wrote:

> Recently I started working on patch for adding additional message from admin
> in pg_terminate_backend for one of our greenplum fork.

Greenplum already has support for passing a message in the terminate command
doesnt it?  Or at least it used to have but perhaps it was ripped out, my
memory is getting a bit fuzzy.

https://github.com/greenplum-db/gpdb-archive/commit/fa6c2d43d675aa05e2d9f797e3008f6fe075ee2c

--
Daniel Gustafsson




Re: Additional message in pg_terminate_backend

От
Roman Khapov
Дата:
> On 17 Dec 2025, at 21:02, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> Greenplum already has support for passing a message in the terminate command
> doesnt it?  Or at least it used to have but perhaps it was ripped out, my
> memory is getting a bit fuzzy.
>
> https://github.com/greenplum-db/gpdb-archive/commit/fa6c2d43d675aa05e2d9f797e3008f6fe075ee2c
>

Well, seems like I missed that patch..

Anyway, now I need same functionality in PostgreSQL, and your patch seems interesting,
especially in part where you keep messages in separated shmem region.

So I adopted your patch, maybe in that form it can be useful for PostgreSQL?







Вложения

Re: Additional message in pg_terminate_backend

От
Kirill Reshke
Дата:
On Sat, 20 Dec 2025 at 14:27, Roman Khapov <rkhapov@yandex-team.ru> wrote:
>
>
> > On 17 Dec 2025, at 21:02, Daniel Gustafsson <daniel@yesql.se> wrote:
> >
> > Greenplum already has support for passing a message in the terminate command
> > doesnt it?  Or at least it used to have but perhaps it was ripped out, my
> > memory is getting a bit fuzzy.
> >
> > https://github.com/greenplum-db/gpdb-archive/commit/fa6c2d43d675aa05e2d9f797e3008f6fe075ee2c
> >
>
> Well, seems like I missed that patch..
>
> Anyway, now I need same functionality in PostgreSQL, and your patch seems interesting,
> especially in part where you keep messages in separated shmem region.
>
> So I adopted your patch, maybe in that form it can be useful for PostgreSQL?
>



>  +CREATE OR REPLACE FUNCTION
> +  pg_terminate_backend_msg(pid integer, timeout int8 DEFAULT 0, msg text DEFAULT '')
> +  RETURNS boolean STRICT VOLATILE LANGUAGE INTERNAL AS 'pg_terminate_backend_msg'
> +  PARALLEL SAFE;

I don't think we need to create a function with a name other than
`pg_terminate_backend`.  I also do not think we need
pg_terminate_backend_msg as a wrapper to another function - all of
this can be a single function, accepting different number of params,
exampli gratia "bt_index_check"

```
Datum
bt_index_check(PG_FUNCTION_ARGS)
{
Oid indrelid = PG_GETARG_OID(0);

...

if (PG_NARGS() >= 2)
args.heapallindexed = PG_GETARG_BOOL(1);
if (PG_NARGS() >= 3)
args.checkunique = PG_GETARG_BOOL(2);
....


PG_RETURN_VOID();
}

```

-- 
Best regards,
Kirill Reshke



Re: Additional message in pg_terminate_backend

От
Jim Jones
Дата:
Hi Roman,

As pointed out by Kirill, there is no reason to create
pg_terminate_backend_msg or pg_cancel_backend_msg. You can simply use
the existing functions and expand them to use one extra parameter, e.g.

CREATE OR REPLACE FUNCTION
  pg_terminate_backend(pid integer, timeout int8 DEFAULT 0, msg text
DEFAULT '')
  RETURNS boolean STRICT VOLATILE LANGUAGE INTERNAL AS
'pg_terminate_backend'
  PARALLEL SAFE;


Here I don't think we need to check PG_NARGS, since the function calls
will always have a default value. Something like this perhaps:


Datum pg_terminate_backend(PG_FUNCTION_ARGS)
{
  int pid;
  int timeout; /* milliseconds */
  char *msg;

  pid = PG_GETARG_INT32(0);
  timeout = PG_GETARG_INT64(1);
  msg = text_to_cstring(PG_GETARG_TEXT_PP(2));

  return pg_terminate_backend_internal(pid, timeout, msg);
}

stpncpy() -> strlcpy()

Documentation is missing -- assuming the feature design is already solid.

Add backend_msg.c to meson.build.

I rebased the patch (it was failing for quite some time) with some
suggestions. Feel free to remove them and revert to your v2 if you disagree.

I'll review the rest of code in the next days.

Best, Jim
Вложения

Re: Additional message in pg_terminate_backend

От
Jim Jones
Дата:

On 01/02/2026 01:08, Jim Jones wrote:
> As pointed out by Kirill, there is no reason to create
> pg_terminate_backend_msg or pg_cancel_backend_msg. You can simply use
> the existing functions and expand them to use one extra parameter

Since the message's size is limited to BACKEND_MSG_MAX_LEN, shouldn't
you use it to limit msg at pg_terminate_backend[_msg]()? Something like:

Datum pg_terminate_backend(PG_FUNCTION_ARGS)
{
  int pid;
  int timeout; /* milliseconds */
  char msg[BACKEND_MSG_MAX_LEN];

  pid = PG_GETARG_INT32(0);
  timeout = PG_GETARG_INT64(1);
  text_to_cstring_buffer(PG_GETARG_TEXT_PP(2), msg, sizeof(msg));

  return pg_terminate_backend_internal(pid, timeout, msg);
}





Re: Additional message in pg_terminate_backend

От
Roman Khapov
Дата:
Hi Jim!
Thanks for your review and rebase!

> Since the message's size is limited to BACKEND_MSG_MAX_LEN, shouldn't
> you use it to limit msg at pg_terminate_backend[_msg]()? Something like:
> 

The message is truncated inside BackendMsgSet function, so I see a little
point in truncating it at pg_terminate_backend..

Also, changing the stpncpy() to strlcpy() breaks the logic
of returning result length of the message and NOTICE message about it,
so I reverted this change. But this note make me think about adding test
to truncation logic, so I added it in v4.

--
Best regards,
Roman Khapov


Вложения

Re: Additional message in pg_terminate_backend

От
Jim Jones
Дата:
On 03/02/2026 08:52, Roman Khapov wrote:
> The message is truncated inside BackendMsgSet function, so I see a little
> point in truncating it at pg_terminate_backend..

Thanks for the update!

You might have a point there. One issue I see is with UTF8 character
boundaries. Calculating len like this can lead to invalid characters,
for instance:

postgres=# SELECT
pg_terminate_backend(pg_backend_pid(),0,repeat('🐘',1000));

NOTICE:  message is too long, truncated to 127
FATAL:  terminating connection due to administrator command:
🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘🐘�
server closed the connection unexpectedly
        This probably means the server terminated abnormally


You might wanna take a look at other alternatives, such as pg_mbcliplen,
for instance (pseudocode):

len = strlen(msg);
if (len >= sizeof(slot->msg))
  len = pg_mbcliplen(msg, len, sizeof(slot->msg) - 1);

memcpy(slot->msg, msg, len);
slot->msg[len] = '\0';

Best, Jim



Re: Additional message in pg_terminate_backend

От
Andrey Borodin
Дата:

> On 3 Feb 2026, at 12:52, Roman Khapov <rkhapov@yandex-team.ru> wrote:
>
> <v4-0001-message-in-pg_terminate_backend-and-pg_cancel_bac.patch>

Some notes on this version:

1. Did you mean BackendMsgShmemSize()?
    size = add_size(size, BackendStatusShmemSize());

2. In docs:

<function>pg_cancel_backend</function> ( <parameter>pid</parameter> <type>integer</type>,
<parameter>message</parameter><type>test</type> <literal>DEFAULT</literal> <literal>''</literal> ) 

Did you mean <type>text</type>?

3. Windows build failed [0]

4. In src/include/utils/misc/backend_msg.c identification is backend_msg.h


Best regards, Andrey Borodin.

[0] https://github.com/x4m/postgres_g/runs/62314358734


Re: Additional message in pg_terminate_backend

От
Roman Khapov
Дата:
Hi again Jim, Andrew!

Thanks for another round on review, updated the patch according to comments.

Also, fix `make check` by updating pg_proc.data with new functions,
similar to uuid7 way: defining _msg versions of the functions

--
Best regards,
Roman Khapov



> On 3 Feb 2026, at 13:26, Andrey Borodin <x4mmm@yandex-team.ru> wrote:
>
>
>
>> On 3 Feb 2026, at 12:52, Roman Khapov <rkhapov@yandex-team.ru> wrote:
>>
>> <v4-0001-message-in-pg_terminate_backend-and-pg_cancel_bac.patch>
>
> Some notes on this version:
>
> 1. Did you mean BackendMsgShmemSize()?
>    size = add_size(size, BackendStatusShmemSize());
>
> 2. In docs:
>
> <function>pg_cancel_backend</function> ( <parameter>pid</parameter> <type>integer</type>,
<parameter>message</parameter><type>test</type> <literal>DEFAULT</literal> <literal>''</literal> ) 
>
> Did you mean <type>text</type>?
>
> 3. Windows build failed [0]
>
> 4. In src/include/utils/misc/backend_msg.c identification is backend_msg.h
>
>
> Best regards, Andrey Borodin.
>
> [0] https://github.com/x4m/postgres_g/runs/62314358734



Вложения

Re: Additional message in pg_terminate_backend

От
Jim Jones
Дата:
Hi Roman

On 03/02/2026 13:28, Roman Khapov wrote:
> Thanks for another round on review, updated the patch according to comments.
> 
> Also, fix `make check` by updating pg_proc.data with new functions,
> similar to uuid7 way: defining _msg versions of the functions


Why did you decide to revert the pg_proc.dat and system_functions.sql
changes from v3? In v3 I thought that adding a msg DEFAULT '' to the
corresponding function in system_functions.sql ...

--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -400,7 +400,12 @@ CREATE OR REPLACE FUNCTION
   PARALLEL SAFE;

 CREATE OR REPLACE FUNCTION
-  pg_terminate_backend(pid integer, timeout int8 DEFAULT 0)
+  pg_cancel_backend(pid integer, msg text DEFAULT '')
+  RETURNS boolean STRICT VOLATILE LANGUAGE INTERNAL AS 'pg_cancel_backend'
+  PARALLEL SAFE;
+
+CREATE OR REPLACE FUNCTION
+  pg_terminate_backend(pid integer, timeout int8 DEFAULT 0, msg text
DEFAULT '')
   RETURNS boolean STRICT VOLATILE LANGUAGE INTERNAL AS
'pg_terminate_backend'
   PARALLEL SAFE;

... and updating pg_proc.dat accordingly would do the trick

diff --git a/src/include/catalog/pg_proc.dat
b/src/include/catalog/pg_proc.dat
index 5e5e33f64f..47aa30d716 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6724,10 +6724,11 @@

 { oid => '2171', descr => 'cancel a server process\' current query',
   proname => 'pg_cancel_backend', provolatile => 'v', prorettype => 'bool',
-  proargtypes => 'int4', prosrc => 'pg_cancel_backend' },
+  proargtypes => 'int4 text', proargnames => '{pid,msg}',
+  prosrc => 'pg_cancel_backend' },
 { oid => '2096', descr => 'terminate a server process',
   proname => 'pg_terminate_backend', provolatile => 'v', prorettype =>
'bool',
-  proargtypes => 'int4 int8', proargnames => '{pid,timeout}',
+  proargtypes => 'int4 int8 text', proargnames => '{pid,timeout,msg}',
   prosrc => 'pg_terminate_backend' },
 { oid => '2172', descr => 'prepare for taking an online backup',
   proname => 'pg_backup_start', provolatile => 'v', proparallel => 'r',


My rationale is based on the header in system_function.sql:

...
 * src/backend/catalog/system_functions.sql
 *
 * This file redefines certain built-in functions that are impractical
 * to fully define in pg_proc.dat.  In most cases that's because they use
 * SQL-standard function bodies and/or default expressions...


Am I missing something?

Thanks

Best, Jim