Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Fri, 23 Jun 2017 19:43:35 -0400 Stephen Frost wrote: > Alvaro, all, > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Yugo Nagata wrote: > > > > > I tried to make it. Patch attached. > > > > > > It is easy and simple. Although I haven't tried for autovacuum worker, > > > I confirmed I can change other process' parameters without affecting > > > others. > > > Do you want this in PG? > > > > One advantage of this gadget is that you can signal backends that you > > own without being superuser, so +1 for the general idea. (Please do > > create a fresh thread so that this can be appropriately linked to in > > commitfest app, though.) > > Well, that wouldn't quite work with the proposed patch because the > proposed patch REVOKE's execute from public... Yes. It is intentional to revoke execute from public because we need to change configuration before signaling other backend and it needs superuser privilege. > I'm trying to figure out how it's actually useful to be able to signal a > backend that you own about a config change that you *aren't* able to > make without being a superuser.. Now, if you could tell the other > backend to use a new value for some USERSET GUC, then *that* would be > useful and interesting. > > I haven't got any particularly great ideas for how to make that happen > though. > > > You need a much bigger patch for the autovacuum worker. The use case I > > had in mind is that you have a maintenance window and can run fast > > vacuuming during it, but if the table is large and vacuum can't finish > > within that window then you want vacuum to slow down, without stopping > > it completely. But implementing this requires juggling the > > stdVacuumCostDelay/Limit values during the reload, which are currently > > read at start of vacuuming only; the working values are overwritten from > > those during a rebalance. > > Being able to change those values during a vacuuming run would certainly > be useful, I've had cases where I would have liked to have been able to > do just exactly that. That's largely independent of this though. > > Thanks! > > Stephen -- Yugo Nagata -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Sat, 24 Jun 2017 08:09:52 +0900 Michael Paquier wrote: > On Sat, Jun 24, 2017 at 5:07 AM, Alvaro Herrera > wrote: > > Yugo Nagata wrote: > > > >> I tried to make it. Patch attached. > >> > >> It is easy and simple. Although I haven't tried for autovacuum worker, > >> I confirmed I can change other process' parameters without affecting > >> others. > >> Do you want this in PG? > > Just browsing the patch... > > +if (r == SIGNAL_BACKEND_NOSUPERUSER) > +ereport(WARNING, > +(errmsg("must be a superuser to terminate superuser > process"))); > + > +if (r == SIGNAL_BACKEND_NOPERMISSION) > +ereport(WARNING, > + (errmsg("must be a member of the role whose process > is being terminated or member of pg_signal_backend"))); > Both messages are incorrect. This is not trying to terminate a process. I'll fix this. > > +Datum > +pg_reload_conf_pid(PG_FUNCTION_ARGS) > I think that the naming is closer to pg_reload_backend. > > Documentation is needed as well. Agree with pg_reload_backend and I'll write the documetation. > > > One advantage of this gadget is that you can signal backends that you > > own without being superuser, so +1 for the general idea. (Please do > > create a fresh thread so that this can be appropriately linked to in > > commitfest app, though.) > > That would be nice. Sure. I'll create a new thread and attach the new patch to it. > > > You need a much bigger patch for the autovacuum worker. The use case I > > had in mind is that you have a maintenance window and can run fast > > vacuuming during it, but if the table is large and vacuum can't finish > > within that window then you want vacuum to slow down, without stopping > > it completely. But implementing this requires juggling the > > stdVacuumCostDelay/Limit values during the reload, which are currently > > read at start of vacuuming only; the working values are overwritten from > > those during a rebalance. > > Yeah, that's independent from the patch above. Agree. It would be another feature from pg_reload_backend and I think it would be implmented as another patch. > -- > 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
Alvaro, all, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Yugo Nagata wrote: > > > I tried to make it. Patch attached. > > > > It is easy and simple. Although I haven't tried for autovacuum worker, > > I confirmed I can change other process' parameters without affecting others. > > Do you want this in PG? > > One advantage of this gadget is that you can signal backends that you > own without being superuser, so +1 for the general idea. (Please do > create a fresh thread so that this can be appropriately linked to in > commitfest app, though.) Well, that wouldn't quite work with the proposed patch because the proposed patch REVOKE's execute from public... I'm trying to figure out how it's actually useful to be able to signal a backend that you own about a config change that you *aren't* able to make without being a superuser.. Now, if you could tell the other backend to use a new value for some USERSET GUC, then *that* would be useful and interesting. I haven't got any particularly great ideas for how to make that happen though. > You need a much bigger patch for the autovacuum worker. The use case I > had in mind is that you have a maintenance window and can run fast > vacuuming during it, but if the table is large and vacuum can't finish > within that window then you want vacuum to slow down, without stopping > it completely. But implementing this requires juggling the > stdVacuumCostDelay/Limit values during the reload, which are currently > read at start of vacuuming only; the working values are overwritten from > those during a rebalance. Being able to change those values during a vacuuming run would certainly be useful, I've had cases where I would have liked to have been able to do just exactly that. That's largely independent of this though. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Sat, Jun 24, 2017 at 5:07 AM, Alvaro Herrera wrote: > Yugo Nagata wrote: > >> I tried to make it. Patch attached. >> >> It is easy and simple. Although I haven't tried for autovacuum worker, >> I confirmed I can change other process' parameters without affecting others. >> Do you want this in PG? Just browsing the patch... +if (r == SIGNAL_BACKEND_NOSUPERUSER) +ereport(WARNING, +(errmsg("must be a superuser to terminate superuser process"))); + +if (r == SIGNAL_BACKEND_NOPERMISSION) +ereport(WARNING, + (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))); Both messages are incorrect. This is not trying to terminate a process. +Datum +pg_reload_conf_pid(PG_FUNCTION_ARGS) I think that the naming is closer to pg_reload_backend. Documentation is needed as well. > One advantage of this gadget is that you can signal backends that you > own without being superuser, so +1 for the general idea. (Please do > create a fresh thread so that this can be appropriately linked to in > commitfest app, though.) That would be nice. > You need a much bigger patch for the autovacuum worker. The use case I > had in mind is that you have a maintenance window and can run fast > vacuuming during it, but if the table is large and vacuum can't finish > within that window then you want vacuum to slow down, without stopping > it completely. But implementing this requires juggling the > stdVacuumCostDelay/Limit values during the reload, which are currently > read at start of vacuuming only; the working values are overwritten from > those during a rebalance. Yeah, that's independent from the patch above. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
Yugo Nagata wrote: > I tried to make it. Patch attached. > > It is easy and simple. Although I haven't tried for autovacuum worker, > I confirmed I can change other process' parameters without affecting others. > Do you want this in PG? One advantage of this gadget is that you can signal backends that you own without being superuser, so +1 for the general idea. (Please do create a fresh thread so that this can be appropriately linked to in commitfest app, though.) You need a much bigger patch for the autovacuum worker. The use case I had in mind is that you have a maintenance window and can run fast vacuuming during it, but if the table is large and vacuum can't finish within that window then you want vacuum to slow down, without stopping it completely. But implementing this requires juggling the stdVacuumCostDelay/Limit values during the reload, which are currently read at start of vacuuming only; the working values are overwritten from those during a rebalance. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Thu, 22 Jun 2017 14:08:30 +0900 Michael Paquier wrote: > On Thu, Jun 22, 2017 at 1:52 PM, Yugo Nagata wrote: > > On Thu, 22 Jun 2017 12:05:19 +0900 > > Michael Paquier wrote: > >> signal-able). Different thought, but I'd love to see a SQL function > >> that allows triggering SIGHUP on a specific process, like an > >> autovacuum worker to change its cost parameters. There is > >> pg_reload_conf() to do so but that's system-wide. > > > > For example, is that like this? > > > > =# alter system set autovacuum_vacuum_cost_delay to 10; > > =# select pg_reload_conf(); > > =# alter system reset autovacuum_vacuum_cost_delay; > > No need to reset the parameter afterwards as this makes it sensible to > updates afterwards, but you have the idea. Note that this is rather > recent, as autovacuum listens to SIGHUP only since a75fb9b3. I tried to make it. Patch attached. It is easy and simple. Although I haven't tried for autovacuum worker, I confirmed I can change other process' parameters without affecting others. Do you want this in PG? [session A (PID:18375)] =# show autovacuum_vacuum_cost_delay; autovacuum_vacuum_cost_delay -- 20ms (1 row) [session B] postgres=# alter system set autovacuum_vacuum_cost_delay to 10; ALTER SYSTEM postgres=# select pg_reload_conf(18375); pg_reload_conf t (1 row) postgres=# show autovacuum_vacuum_cost_delay; autovacuum_vacuum_cost_delay -- 20ms (1 row) [session A again] postgres=# show autovacuum_vacuum_cost_delay; autovacuum_vacuum_cost_delay -- 10ms (1 row) > -- > Michael -- Yugo Nagata diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 0fdad0c..d79faf7 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1128,6 +1128,7 @@ REVOKE EXECUTE ON FUNCTION pg_wal_replay_pause() FROM public; REVOKE EXECUTE ON FUNCTION pg_wal_replay_resume() FROM public; REVOKE EXECUTE ON FUNCTION pg_rotate_logfile() FROM public; REVOKE EXECUTE ON FUNCTION pg_reload_conf() FROM public; +REVOKE EXECUTE ON FUNCTION pg_reload_conf(int) FROM public; REVOKE EXECUTE ON FUNCTION pg_current_logfile() FROM public; REVOKE EXECUTE ON FUNCTION pg_current_logfile(text) FROM public; diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 62341b8..cc173ba 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -339,6 +339,36 @@ pg_reload_conf(PG_FUNCTION_ARGS) PG_RETURN_BOOL(true); } +/* + * Signal to reload the database configuration for a specified backend + * + * Permission checking for this function is managed through the normal + * GRANT system. + */ +Datum +pg_reload_conf_pid(PG_FUNCTION_ARGS) +{ + int pid = PG_GETARG_INT32(0); + int r = pg_signal_backend(pid, SIGHUP); + + if (r == SIGNAL_BACKEND_NOSUPERUSER) + ereport(WARNING, +(errmsg("must be a superuser to terminate superuser process"))); + + if (r == SIGNAL_BACKEND_NOPERMISSION) + ereport(WARNING, + (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))); + + if (r) + { + ereport(WARNING, +(errmsg("failed to send signal to backend: %d", pid))); + PG_RETURN_BOOL(false); + } + + PG_RETURN_BOOL(true); +} + /* * Rotate log file diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 1191b4a..7258f15 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -3255,6 +3255,8 @@ DESCR("true if wal replay is paused"); DATA(insert OID = 2621 ( pg_reload_conf PGNSP PGUID 12 1 0 0 0 f f f f t f v s 0 0 16 "" _null_ _null_ _null_ _null_ _null_ pg_reload_conf _null_ _null_ _null_ )); DESCR("reload configuration files"); +DATA(insert OID = 3409 ( pg_reload_conf PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 16 "23" _null_ _null_ _null_ _null_ _null_ pg_reload_conf_pid _null_ _null_ _null_ )); +DESCR("reload configuration files"); DATA(insert OID = 2622 ( pg_rotate_logfile PGNSP PGUID 12 1 0 0 0 f f f f t f v s 0 0 16 "" _null_ _null_ _null_ _null_ _null_ pg_rotate_logfile _null_ _null_ _null_ )); DESCR("rotate log file"); DATA(insert OID = 3800 ( pg_current_logfilePGNSP PGUID 12 1 0 0 0 f f f f f f v s 0 0 25 "" _null_ _null_ _null_ _null_ _null_ pg_current_logfile _null_ _null_ _null_ )); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Thu, Jun 22, 2017 at 1:52 PM, Yugo Nagata wrote: > On Thu, 22 Jun 2017 12:05:19 +0900 > Michael Paquier wrote: >> signal-able). Different thought, but I'd love to see a SQL function >> that allows triggering SIGHUP on a specific process, like an >> autovacuum worker to change its cost parameters. There is >> pg_reload_conf() to do so but that's system-wide. > > For example, is that like this? > > =# alter system set autovacuum_vacuum_cost_delay to 10; > =# select pg_reload_conf(); > =# alter system reset autovacuum_vacuum_cost_delay; No need to reset the parameter afterwards as this makes it sensible to updates afterwards, but you have the idea. Note that this is rather recent, as autovacuum listens to SIGHUP only since a75fb9b3. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Thu, 22 Jun 2017 12:05:19 +0900 Michael Paquier wrote: > On Thu, Jun 22, 2017 at 11:52 AM, Andres Freund wrote: > > On 2017-06-22 11:49:47 +0900, Yugo Nagata wrote: > >> I agree that we can kill theses processes by the OS command. However, > >> It seems to me that pg_{cancel,terminate}_backend don't need to be able to > >> kill processes except for client backends because we can do same thing by > >> the OS command if necessary, and acutually these functions cannot kill > >> most other processes, for example, background writer. Are the autovacuum > >> launcher and background worker special for these functions? > > > > I strongly disagree with this - I think it's quite useful to be able to > > kill things via SQL that can hold lock on database objects. I'm not > > seeing which problem would be solved by prohibiting this? > > +1. Controlling them as of now is useful, particularly now that all > processes show wait events, even auxiliary ones (though not I got it. I agree that the current behaviour and it isn't worth changint it. In my understand, processes that the functions can kill (client backends, background workers, autovacuum processes) are ones that can hold lock on database objects. > signal-able). Different thought, but I'd love to see a SQL function > that allows triggering SIGHUP on a specific process, like an > autovacuum worker to change its cost parameters. There is > pg_reload_conf() to do so but that's system-wide. For example, is that like this? =# alter system set autovacuum_vacuum_cost_delay to 10; =# select pg_reload_conf(); =# alter system reset autovacuum_vacuum_cost_delay; > -- > Michael -- Yugo Nagata -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Thu, Jun 22, 2017 at 11:52 AM, Andres Freund wrote: > On 2017-06-22 11:49:47 +0900, Yugo Nagata wrote: >> I agree that we can kill theses processes by the OS command. However, >> It seems to me that pg_{cancel,terminate}_backend don't need to be able to >> kill processes except for client backends because we can do same thing by >> the OS command if necessary, and acutually these functions cannot kill >> most other processes, for example, background writer. Are the autovacuum >> launcher and background worker special for these functions? > > I strongly disagree with this - I think it's quite useful to be able to > kill things via SQL that can hold lock on database objects. I'm not > seeing which problem would be solved by prohibiting this? +1. Controlling them as of now is useful, particularly now that all processes show wait events, even auxiliary ones (though not signal-able). Different thought, but I'd love to see a SQL function that allows triggering SIGHUP on a specific process, like an autovacuum worker to change its cost parameters. There is pg_reload_conf() to do so but that's system-wide. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On 2017-06-22 11:49:47 +0900, Yugo Nagata wrote: > On Wed, 21 Jun 2017 11:04:34 -0400 > Robert Haas wrote: > > > On Wed, Jun 21, 2017 at 7:56 AM, Yugo Nagata wrote: > > > I have found that we can cancel/terminate autovacuum launchers and > > > background worker processes by pg_cancel/terminate_backend function. > > > I'm wondering this behavior is not expected and if not I want to fix it. > > > > I think it is expected. Even if we blocked it, those processes have > > to cope gracefully with SIGTERM, because anyone with access to the OS > > user can kill them that way by hand. > > I agree that we can kill theses processes by the OS command. However, > It seems to me that pg_{cancel,terminate}_backend don't need to be able to > kill processes except for client backends because we can do same thing by > the OS command if necessary, and acutually these functions cannot kill > most other processes, for example, background writer. Are the autovacuum > launcher and background worker special for these functions? I strongly disagree with this - I think it's quite useful to be able to kill things via SQL that can hold lock on database objects. I'm not seeing which problem would be solved by prohibiting this? - Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Wed, 21 Jun 2017 11:04:34 -0400 Robert Haas wrote: > On Wed, Jun 21, 2017 at 7:56 AM, Yugo Nagata wrote: > > I have found that we can cancel/terminate autovacuum launchers and > > background worker processes by pg_cancel/terminate_backend function. > > I'm wondering this behavior is not expected and if not I want to fix it. > > I think it is expected. Even if we blocked it, those processes have > to cope gracefully with SIGTERM, because anyone with access to the OS > user can kill them that way by hand. I agree that we can kill theses processes by the OS command. However, It seems to me that pg_{cancel,terminate}_backend don't need to be able to kill processes except for client backends because we can do same thing by the OS command if necessary, and acutually these functions cannot kill most other processes, for example, background writer. Are the autovacuum launcher and background worker special for these functions? > > > However, we can terminate background workers by pg_terminate_backend. > > In the following example, I terminated the logical replication launcher, > > and this process did not appear again[1]. > > > > postgres=# select pg_terminate_backend(30902); > > pg_terminate_backend > > -- > > t > > (1 row) > > That seems to be a bug in logical replication. > > > Similarly, we can terminate autovacuum launcher by pg_terminate_backend, > > but a new process is restarted by postmaster in this case.[2] > > > > postgres=# select pg_terminate_backend(30900); > > pg_terminate_backend > > -- > > t > > (1 row) > > That is as I would expect. > > > [2] > > On the other hand, when we use pg_cancel_backend for autovacuum launcher, > > it causes the following error. I'll report the detail in another thread. > > > > ERROR: can't attach the same segment more than once > > I think that's a bug. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company -- Yugo Nagata -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
Yugo Nagata wrote: > However, we can terminate background workers by pg_terminate_backend. > In the following example, I terminated the logical replication launcher, > and this process did not appear again[1]. > > postgres=# select pg_terminate_backend(30902); > pg_terminate_backend > -- > t > (1 row) I think failing to restart the replication launcher after it stops is probably a bug, and should be fixed by having postmaster start another one if it dies. > Similarly, we can terminate autovacuum launcher by pg_terminate_backend, > but a new process is restarted by postmaster in this case.[2] Yeah, this is operating as intended. You can turn autovacuum off and the launcher should go away, and turn it back on and launcher should start. So we expect the autovac launcher to respond to signals. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend can terminate background workers and autovacuum launchers
On Wed, Jun 21, 2017 at 7:56 AM, Yugo Nagata wrote: > I have found that we can cancel/terminate autovacuum launchers and > background worker processes by pg_cancel/terminate_backend function. > I'm wondering this behavior is not expected and if not I want to fix it. I think it is expected. Even if we blocked it, those processes have to cope gracefully with SIGTERM, because anyone with access to the OS user can kill them that way by hand. > However, we can terminate background workers by pg_terminate_backend. > In the following example, I terminated the logical replication launcher, > and this process did not appear again[1]. > > postgres=# select pg_terminate_backend(30902); > pg_terminate_backend > -- > t > (1 row) That seems to be a bug in logical replication. > Similarly, we can terminate autovacuum launcher by pg_terminate_backend, > but a new process is restarted by postmaster in this case.[2] > > postgres=# select pg_terminate_backend(30900); > pg_terminate_backend > -- > t > (1 row) That is as I would expect. > [2] > On the other hand, when we use pg_cancel_backend for autovacuum launcher, > it causes the following error. I'll report the detail in another thread. > > ERROR: can't attach the same segment more than once I think that's a bug. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers