Re: [HACKERS] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

2017-06-29 Thread Dmitry Dolgov 
> On 29 Jun 2017 08:18, "Yugo Nagata"  wrote:
>
> This is because this function is defined as strict

Yes, I realized that few moments after I sent my message.

> Arg type is needed.
>
> =# grant execute on function pg_reload_backend(int) to test_user;

Oh, I see, thank you.

Re: [HACKERS] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

2017-06-29 Thread Yugo Nagata 
On Wed, 28 Jun 2017 13:59:51 +0200
Remi Colinet  wrote:

> Hi Yugo,
> 
> The patch looks fine. Installed and tested.

Thank you for your interest.

> 
> But a similar function already exists for the Postmaster process.
> 
> Datum
> pg_reload_conf(PG_FUNCTION_ARGS)
> {
> if (kill(PostmasterPid, SIGHUP))
> {
> ereport(WARNING,
> (errmsg("failed to send signal to
> postmaster: %m")));
> PG_RETURN_BOOL(false);
> }
> 
> PG_RETURN_BOOL(true);
> }
> 
> I would suggest to merge your patch with above function which is close to
> yours.
> Btw, your patch looks better that above function code.
> 
> You may for instance retain pg_reload_conf() without argument for the same
> purpose as currently (Postmaster signaling).
> And provide a pid argument to signal a given backend.

Actually, I proposed that version patch in the previous thread[1], but there
as a suggestion to use other function name, so I attached fixed version
in this thread. 

[1] 
https://www.postgresql.org/message-id/20170624000156.74ca9485.nagata%40sraoss.co.jp

> 
> Regards
> Remi
> 
> 
> 2017-06-28 12:17 GMT+02:00 Yugo Nagata :
> 
> > Hi,
> >
> > Attached is a patch of pg_reload_backend that is a function signaling
> > SIGHUP to a specific backend. The original idea is from Michael Paquier[1].
> > The documatation isn't included in this patch yet.
> >
> > We can change some parameters of other backend using the function
> > as bellow.
> >
> > postgres=# alter system set log_min_messages to debug2;
> > ALTER SYSTEM
> > postgres=# select pg_reload_backend(18375);
> >  pg_reload_backend
> > ---
> >  t
> > (1 row)
> >
> > There are some usecases. For example:
> >
> > - changing log configuration in other backends temporally.
> > - changing cost parameters for planner in other backends.
> > - changing autovacuum parameters of an already running autovacuum worker.
> >
> >
> > Hoever, this function need the superuser previlige to execute as well as
> > pg_reload_conf(). Although we can grant the execution to public, it is
> > useless for normal users bacause only superuser can change postgresql.conf.
> >
> > To allow normal users to change parameters in ohter backends, instead
> > we might want another feature, for example, a function like set_config()
> > than can change other backend's parameters using shared memory and SIGUSR1.
> >
> > Any comments would be appreciated.
> >
> > Regards,
> >
> > [1]
> > https://www.postgresql.org/message-id/CAB7nPqT4y8-
> > QoGKEugk99_tFuEOtAshcs5kxOeZ_0w27UtdGyA%40mail.gmail.com
> >
> > --
> > 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
> >
> >


-- 
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] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

2017-06-29 Thread Yugo Nagata 
On Wed, 28 Jun 2017 13:35:12 +0200
Dmitry Dolgov <9erthali...@gmail.com> wrote:

> > On 28 June 2017 at 12:17, Yugo Nagata  wrote:
> >
> > Hi,
> >
> > Attached is a patch of pg_reload_backend that is a function signaling
> > SIGHUP to a specific backend. The original idea is from Michael
> Paquier[1].
> > The documatation isn't included in this patch yet.
> 
> I have few questions. I'm curious, why this function returns something
> different from bool when I'm passing null as an argument?

This is because this function is defined as strict, that is pg_proc.proisstrict
is true, as well as pg_reload_conf, pg_terminate_backend, pg_cancel_bacnend, 
and so on.

> 
> =# select pg_reload_backend(27961);
> WARNING:  PID 27961 is not a PostgreSQL server process
> WARNING:  failed to send signal to backend: 27961
>  pg_reload_backend
> ---
>  f
> (1 row)
> 
> =# select pg_reload_backend(27962);
>  pg_reload_backend
> ---
>  t
> (1 row)
> 
> =# select pg_reload_backend(null);
>  pg_reload_backend
> ---
> 
> (1 row)
> 
> Also for some reason I can't grant an execute permission on this function,
> am I doing something wrong?
> 
> =# grant execute on function pg_reload_backend() to test_user;
> ERROR:  function pg_reload_backend() does not exist
> =# grant execute on function pg_reload_conf() to test_user;
> GRANT

Arg type is needed.

=# grant execute on function pg_reload_backend(int) to test_user;

-- 
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] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

2017-06-28 Thread Remi Colinet 
Hi Yugo,

The patch looks fine. Installed and tested.

But a similar function already exists for the Postmaster process.

Datum
pg_reload_conf(PG_FUNCTION_ARGS)
{
if (kill(PostmasterPid, SIGHUP))
{
ereport(WARNING,
(errmsg("failed to send signal to
postmaster: %m")));
PG_RETURN_BOOL(false);
}

PG_RETURN_BOOL(true);
}

I would suggest to merge your patch with above function which is close to
yours.
Btw, your patch looks better that above function code.

You may for instance retain pg_reload_conf() without argument for the same
purpose as currently (Postmaster signaling).
And provide a pid argument to signal a given backend.

Regards
Remi


2017-06-28 12:17 GMT+02:00 Yugo Nagata :

> Hi,
>
> Attached is a patch of pg_reload_backend that is a function signaling
> SIGHUP to a specific backend. The original idea is from Michael Paquier[1].
> The documatation isn't included in this patch yet.
>
> We can change some parameters of other backend using the function
> as bellow.
>
> postgres=# alter system set log_min_messages to debug2;
> ALTER SYSTEM
> postgres=# select pg_reload_backend(18375);
>  pg_reload_backend
> ---
>  t
> (1 row)
>
> There are some usecases. For example:
>
> - changing log configuration in other backends temporally.
> - changing cost parameters for planner in other backends.
> - changing autovacuum parameters of an already running autovacuum worker.
>
>
> Hoever, this function need the superuser previlige to execute as well as
> pg_reload_conf(). Although we can grant the execution to public, it is
> useless for normal users bacause only superuser can change postgresql.conf.
>
> To allow normal users to change parameters in ohter backends, instead
> we might want another feature, for example, a function like set_config()
> than can change other backend's parameters using shared memory and SIGUSR1.
>
> Any comments would be appreciated.
>
> Regards,
>
> [1]
> https://www.postgresql.org/message-id/CAB7nPqT4y8-
> QoGKEugk99_tFuEOtAshcs5kxOeZ_0w27UtdGyA%40mail.gmail.com
>
> --
> 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] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

2017-06-28 Thread Dmitry Dolgov 
> On 28 June 2017 at 12:17, Yugo Nagata  wrote:
>
> Hi,
>
> Attached is a patch of pg_reload_backend that is a function signaling
> SIGHUP to a specific backend. The original idea is from Michael
Paquier[1].
> The documatation isn't included in this patch yet.

I have few questions. I'm curious, why this function returns something
different from bool when I'm passing null as an argument?

=# select pg_reload_backend(27961);
WARNING:  PID 27961 is not a PostgreSQL server process
WARNING:  failed to send signal to backend: 27961
 pg_reload_backend
---
 f
(1 row)

=# select pg_reload_backend(27962);
 pg_reload_backend
---
 t
(1 row)

=# select pg_reload_backend(null);
 pg_reload_backend
---

(1 row)

Also for some reason I can't grant an execute permission on this function,
am I doing something wrong?

=# grant execute on function pg_reload_backend() to test_user;
ERROR:  function pg_reload_backend() does not exist
=# grant execute on function pg_reload_conf() to test_user;
GRANT

[HACKERS] [PATCH] pg_reload_backend to signal SIGHUP to a specific backend

2017-06-28 Thread Yugo Nagata 
Hi,

Attached is a patch of pg_reload_backend that is a function signaling
SIGHUP to a specific backend. The original idea is from Michael Paquier[1].
The documatation isn't included in this patch yet.

We can change some parameters of other backend using the function
as bellow.

postgres=# alter system set log_min_messages to debug2;
ALTER SYSTEM
postgres=# select pg_reload_backend(18375);
 pg_reload_backend 
---
 t
(1 row)

There are some usecases. For example:

- changing log configuration in other backends temporally.
- changing cost parameters for planner in other backends.
- changing autovacuum parameters of an already running autovacuum worker.


Hoever, this function need the superuser previlige to execute as well as
pg_reload_conf(). Although we can grant the execution to public, it is
useless for normal users bacause only superuser can change postgresql.conf.

To allow normal users to change parameters in ohter backends, instead
we might want another feature, for example, a function like set_config()
than can change other backend's parameters using shared memory and SIGUSR1.

Any comments would be appreciated.

Regards,

[1]
https://www.postgresql.org/message-id/CAB7nPqT4y8-QoGKEugk99_tFuEOtAshcs5kxOeZ_0w27UtdGyA%40mail.gmail.com

-- 
Yugo Nagata 
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 0fdad0c..3a9a020 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_backend(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..83b6c45 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_backend(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 reload superuser process")));
+
+	if (r == SIGNAL_BACKEND_NOPERMISSION)
+		ereport(WARNING,
+ (errmsg("must be a member of the role whose process is being reloaded 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..a05e78a 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_backend			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_backend _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