proposal: psql: psql variable BACKEND_PID

Started by Pavel Stehuleabout 3 years ago27 messageshackers
Jump to latest
#1Pavel Stehule
pavel.stehule@gmail.com

Hi

We can simply allow an access to backend process id thru psql variable. I
propose the name "BACKEND_PID". The advantages of usage are simple
accessibility by command \set, and less typing then using function
pg_backend_pid, because psql variables are supported by tab complete
routine. Implementation is very simple, because we can use the function
PQbackendPID.

Comments, notes?

Regards

Pavel

Attachments:

0001-implementation-of-BACKEND_PID-psql-s-variable.patchtext/x-patch; charset=US-ASCII; name=0001-implementation-of-BACKEND_PID-psql-s-variable.patchDownload+5-1
#2Corey Huinker
corey.huinker@gmail.com
In reply to: Pavel Stehule (#1)
Re: proposal: psql: psql variable BACKEND_PID

On Fri, Feb 3, 2023 at 5:42 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

We can simply allow an access to backend process id thru psql variable. I
propose the name "BACKEND_PID". The advantages of usage are simple
accessibility by command \set, and less typing then using function
pg_backend_pid, because psql variables are supported by tab complete
routine. Implementation is very simple, because we can use the function
PQbackendPID.

Comments, notes?

Regards

Pavel

Interesting, and probably useful.

It needs a corresponding line in UnsyncVariables():

SetVariable(pset.vars, "BACKEND_PID", NULL);

That will set the variable back to null when the connection goes away.

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Corey Huinker (#2)
Re: proposal: psql: psql variable BACKEND_PID

Hi

pá 3. 2. 2023 v 20:27 odesílatel Corey Huinker <corey.huinker@gmail.com>
napsal:

On Fri, Feb 3, 2023 at 5:42 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

We can simply allow an access to backend process id thru psql variable. I
propose the name "BACKEND_PID". The advantages of usage are simple
accessibility by command \set, and less typing then using function
pg_backend_pid, because psql variables are supported by tab complete
routine. Implementation is very simple, because we can use the function
PQbackendPID.

Comments, notes?

Regards

Pavel

Interesting, and probably useful.

It needs a corresponding line in UnsyncVariables():

SetVariable(pset.vars, "BACKEND_PID", NULL);

That will set the variable back to null when the connection goes away.

with doc and unsetting variable

Regards

Pavel

Show quoted text

Attachments:

v20230204-0001-implementation-of-BACKEND_PID-psql-s-variable.patchtext/x-patch; charset=US-ASCII; name=v20230204-0001-implementation-of-BACKEND_PID-psql-s-variable.patchDownload+17-1
#4Corey Huinker
corey.huinker@gmail.com
In reply to: Pavel Stehule (#3)
Re: proposal: psql: psql variable BACKEND_PID

with doc and unsetting variable

Regards

Pavel

Patch applies.

Manually testing confirms that it works, at least for the connected state.
I don't actually know how get psql to invoke DISCONNECT, so I killed the
dev server and can confirm

[222281:14:57:01 EST] corey=# \echo :BACKEND_PID
222281
[222281:14:57:04 EST] corey=# select 1;
FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
Time: 1.554 ms
[:15:02:31 EST] !> \echo :BACKEND_PID
:BACKEND_PID

Clearly, it is hard to write a regression test for an externally volatile
value. `SELECT sign(:BACKEND_PID)` would technically do the job, if we're
striving for completeness.

The inability to easily DISCONNECT via psql, and the deleterious effect
that would have on other regression tests tells me that we can leave that
one untested.

Notes:

This effectively makes the %p prompt (which I use in the example above) the
same as %:BACKEND_PID: and we may want to note that in the documentation.

Do we want to change %p to pull from this variable and save the snprintf()?
Not a measurable savings, more or a don't-repeat-yourself thing.

In the varlistentry, I suggest we add "This variable is unset when the
connection is lost." after "but can be changed or unset.

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Corey Huinker (#4)
Re: proposal: psql: psql variable BACKEND_PID

Hi

so 4. 2. 2023 v 21:36 odesílatel Corey Huinker <corey.huinker@gmail.com>
napsal:

with doc and unsetting variable

Regards

Pavel

Patch applies.

Manually testing confirms that it works, at least for the connected state.
I don't actually know how get psql to invoke DISCONNECT, so I killed the
dev server and can confirm

[222281:14:57:01 EST] corey=# \echo :BACKEND_PID
222281
[222281:14:57:04 EST] corey=# select 1;
FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
Time: 1.554 ms
[:15:02:31 EST] !> \echo :BACKEND_PID
:BACKEND_PID

Clearly, it is hard to write a regression test for an externally volatile
value. `SELECT sign(:BACKEND_PID)` would technically do the job, if we're
striving for completeness.

I did simple test - :BACKEND_PID should be equal pg_backend_pid()

The inability to easily DISCONNECT via psql, and the deleterious effect
that would have on other regression tests tells me that we can leave that
one untested.

I agree

Notes:

This effectively makes the %p prompt (which I use in the example above)
the same as %:BACKEND_PID: and we may want to note that in the
documentation.

done

Do we want to change %p to pull from this variable and save the
snprintf()? Not a measurable savings, more or a don't-repeat-yourself thing.

I checked the code, and I don't think so. Current state is safer (I think).
The psql's variables are not protected, and I think, so is safer, better to
read the value for prompt directly by usage of the libpq API instead read
the possibly "customized" variable. I see possible inconsistency,
but again, the same inconsistency can be for variables USER and DBNAME too,
and I am not able to say what is significantly better. Just prompt shows
real value, and the related variable is +/- copy in connection time.

I am not 100% sure in this area what is better, but the change requires
wider and more general discussion, and I don't think the benefits of
possible
change are enough. There are just two possible solutions - we can protect
some psql's variables (and it can do some compatibility issues), or we
need to accept, so the value in prompt can be fake. It is better to not
touch it :-).

In the varlistentry, I suggest we add "This variable is unset when the
connection is lost." after "but can be changed or unset.

done

Regards

Pavel

Attachments:

v20230205-0001-implementation-of-BACKEND_PID-psql-s-variable.patchtext/x-patch; charset=US-ASCII; name=v20230205-0001-implementation-of-BACKEND_PID-psql-s-variable.patchDownload+31-2
#6Corey Huinker
corey.huinker@gmail.com
In reply to: Pavel Stehule (#5)
Re: proposal: psql: psql variable BACKEND_PID

Clearly, it is hard to write a regression test for an externally volatile
value. `SELECT sign(:BACKEND_PID)` would technically do the job, if we're
striving for completeness.

I did simple test - :BACKEND_PID should be equal pg_backend_pid()

Even better.

Do we want to change %p to pull from this variable and save the
snprintf()? Not a measurable savings, more or a don't-repeat-yourself thing.

I checked the code, and I don't think so. Current state is safer (I
think). The psql's variables are not protected, and I think, so is safer,
better to
read the value for prompt directly by usage of the libpq API instead read
the possibly "customized" variable. I see possible inconsistency,
but again, the same inconsistency can be for variables USER and DBNAME
too, and I am not able to say what is significantly better. Just prompt
shows
real value, and the related variable is +/- copy in connection time.

I am not 100% sure in this area what is better, but the change requires
wider and more general discussion, and I don't think the benefits of
possible
change are enough. There are just two possible solutions - we can protect
some psql's variables (and it can do some compatibility issues), or we
need to accept, so the value in prompt can be fake. It is better to not
touch it :-).

I agree it is out of scope of this patch, but I like the idea of protected
psql variables, and I doubt users are intentionally re-using these vars to
any positive effect. The more likely case is that newer psql vars just
happen to use the names chosen by somebody's script in the past.

done

Everything passes. Docs look good. Test looks good.

#7Corey Huinker
corey.huinker@gmail.com
In reply to: Corey Huinker (#6)
Re: proposal: psql: psql variable BACKEND_PID

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed

A small but helpful feature.

The new status of this patch is: Ready for Committer

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Corey Huinker (#6)
Re: proposal: psql: psql variable BACKEND_PID

po 6. 2. 2023 v 0:25 odesílatel Corey Huinker <corey.huinker@gmail.com>
napsal:

Clearly, it is hard to write a regression test for an externally
volatile value. `SELECT sign(:BACKEND_PID)` would technically do the job,
if we're striving for completeness.

I did simple test - :BACKEND_PID should be equal pg_backend_pid()

Even better.

Do we want to change %p to pull from this variable and save the
snprintf()? Not a measurable savings, more or a don't-repeat-yourself thing.

I checked the code, and I don't think so. Current state is safer (I
think). The psql's variables are not protected, and I think, so is safer,
better to
read the value for prompt directly by usage of the libpq API instead read
the possibly "customized" variable. I see possible inconsistency,
but again, the same inconsistency can be for variables USER and DBNAME
too, and I am not able to say what is significantly better. Just prompt
shows
real value, and the related variable is +/- copy in connection time.

I am not 100% sure in this area what is better, but the change requires
wider and more general discussion, and I don't think the benefits of
possible
change are enough. There are just two possible solutions - we can protect
some psql's variables (and it can do some compatibility issues), or we
need to accept, so the value in prompt can be fake. It is better to not
touch it :-).

I agree it is out of scope of this patch, but I like the idea of protected
psql variables, and I doubt users are intentionally re-using these vars to
any positive effect. The more likely case is that newer psql vars just
happen to use the names chosen by somebody's script in the past.

bash variables are not protected too. I know this is in a different
context, and different architecture. It can be a very simple patch, but it
needs wider discussion. Probably it should be immutable, it is safer, and
now I do not have any useful use case for mutability of these variables.

Regards

Pavel

Show quoted text

done

Everything passes. Docs look good. Test looks good.

#9Pavel Stehule
pavel.stehule@gmail.com
In reply to: Corey Huinker (#7)
Re: proposal: psql: psql variable BACKEND_PID

po 6. 2. 2023 v 0:35 odesílatel Corey Huinker <corey.huinker@gmail.com>
napsal:

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed

A small but helpful feature.

The new status of this patch is: Ready for Committer

Thank you very much

Pavel

#10Daniel Verite
daniel@manitou-mail.org
In reply to: Corey Huinker (#4)
Re: proposal: psql: psql variable BACKEND_PID

Corey Huinker wrote:

Manually testing confirms that it works, at least for the connected state. I
don't actually know how get psql to invoke DISCONNECT, so I killed the dev
server and can confirm

Maybe something like this could be used, with no external action:

postgres=# \echo :BACKEND_PID
10805
postgres=# create user tester superuser;
CREATE ROLE
postgres=# \c postgres tester
You are now connected to database "postgres" as user "tester".
postgres=# alter user tester nosuperuser connection limit 0;
ALTER ROLE
postgres=# select pg_terminate_backend(pg_backend_pid());
FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.

!?> \echo :BACKEND_PID
:BACKEND_PID

In the varlistentry, I suggest we add "This variable is unset when the
connection is lost." after "but can be changed or unset.

Personally I'd much rather have BACKEND_PID set to 0 rather than being unset
when not connected. For one thing it allows safely using \if :BACKEND_PID.

Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite

#11Daniel Verite
daniel@manitou-mail.org
In reply to: Daniel Verite (#10)
Re: proposal: psql: psql variable BACKEND_PID

I wrote:

In the varlistentry, I suggest we add "This variable is unset when the
connection is lost." after "but can be changed or unset.

Personally I'd much rather have BACKEND_PID set to 0 rather than being unset
when not connected. For one thing it allows safely using \if :BACKEND_PID.

Oops it turns out that was wishful thinking from me.
\if does not interpret a non-zero integer as true, except for the
value "1".
I'd still prefer BACKEND_PID being 0 when not connected, though.

Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite

#12Pavel Stehule
pavel.stehule@gmail.com
In reply to: Daniel Verite (#11)
Re: proposal: psql: psql variable BACKEND_PID

po 6. 2. 2023 v 13:03 odesílatel Daniel Verite <daniel@manitou-mail.org>
napsal:

I wrote:

In the varlistentry, I suggest we add "This variable is unset when the
connection is lost." after "but can be changed or unset.

Personally I'd much rather have BACKEND_PID set to 0 rather than being

unset

when not connected. For one thing it allows safely using \if

:BACKEND_PID.

Oops it turns out that was wishful thinking from me.
\if does not interpret a non-zero integer as true, except for the
value "1".
I'd still prefer BACKEND_PID being 0 when not connected, though.

I think psql never tries to execute a query if the engine is not connected,
so for usage in queries undefined state is not important - it will be
always defined.

for using in \if is unset may be a better state, because you can try to use
{? varname} syntax.

0 is theoretically valid process id number, so I am not sure if 0 is ok. I
don't know if some numbers can be used like invalid process id?

Show quoted text

Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite

#13Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#1)
Re: proposal: psql: psql variable BACKEND_PID

On 03.02.23 11:41, Pavel Stehule wrote:

We can simply allow an access to backend process id thru psql variable.
I propose the name "BACKEND_PID". The advantages of usage are simple
accessibility by command \set, and less typing then using function
pg_backend_pid, because psql variables are supported by tab complete
routine. Implementation is very simple, because we can use the function
PQbackendPID.

What would this be useful for?

You can mostly do this using

select pg_backend_pid() AS "BACKEND_PID" \gset

#14Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#13)
Re: proposal: psql: psql variable BACKEND_PID

čt 9. 2. 2023 v 9:57 odesílatel Peter Eisentraut <
peter.eisentraut@enterprisedb.com> napsal:

On 03.02.23 11:41, Pavel Stehule wrote:

We can simply allow an access to backend process id thru psql variable.
I propose the name "BACKEND_PID". The advantages of usage are simple
accessibility by command \set, and less typing then using function
pg_backend_pid, because psql variables are supported by tab complete
routine. Implementation is very simple, because we can use the function
PQbackendPID.

What would this be useful for?

You can mostly do this using

select pg_backend_pid() AS "BACKEND_PID" \gset

there are 2 (3) my motivations

first and main (for me) - I can use psql variables tab complete - just
:B<tab> - it is significantly faster
second - I can see all connection related information by \set
third - there is not hook on reconnect in psql - so if you implement
BACKEND_PID by self, you ensure to run query with pg_backend_pid() after
any reconnect or connection change.

It is clean so you can run "select pg_backend_pid() AS "BACKEND_PID" \gset"
and you can store it to .psqlrc. But most of the time I am in customer's
environment, and I have the time, possibility to do a complete setup of
.psqlrc. It looks (for me) like a generally useful feature to be
everywhere.

Regards

Pavel

#15Andres Freund
andres@anarazel.de
In reply to: Pavel Stehule (#14)
Re: proposal: psql: psql variable BACKEND_PID

Hi,

On 2023-02-09 10:11:21 +0100, Pavel Stehule wrote:

first and main (for me) - I can use psql variables tab complete - just
:B<tab> - it is significantly faster
second - I can see all connection related information by \set
third - there is not hook on reconnect in psql - so if you implement
BACKEND_PID by self, you ensure to run query with pg_backend_pid() after
any reconnect or connection change.

It is clean so you can run "select pg_backend_pid() AS "BACKEND_PID" \gset"
and you can store it to .psqlrc. But most of the time I am in customer's
environment, and I have the time, possibility to do a complete setup of
.psqlrc. It looks (for me) like a generally useful feature to be
everywhere.

I personally just solved this by using %p in PROMPT*. Not that that serves
quite the same niche.

I guess the fact that we have %p is a minor precedent of psql special casing
backend pid in psql.

Greetings,

Andres Freund

#16Andres Freund
andres@anarazel.de
In reply to: Corey Huinker (#4)
Re: proposal: psql: psql variable BACKEND_PID

On 2023-02-04 15:35:58 -0500, Corey Huinker wrote:

This effectively makes the %p prompt (which I use in the example above) the
same as %:BACKEND_PID: and we may want to note that in the documentation.

I don't really see much of a point in noting this in the doc. I don't know in
what situation a user would be helped by reading

+         This substitution is almost equal to using <literal>%:BACKEND_PID:</literal>,
+         but it is safer, because psql variable can be overwriten or unset.

or just about any reformulation of that?

#17Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#14)
Re: proposal: psql: psql variable BACKEND_PID

On 09.02.23 10:11, Pavel Stehule wrote:

first and main (for me) - I can use psql variables tab complete - just
:B<tab> - it is significantly faster
second - I can see all connection related information by \set
third - there is not hook on reconnect in psql - so if you implement
BACKEND_PID by self, you ensure to run query with pg_backend_pid() after
any reconnect or connection change.

It is clean so you can run "select pg_backend_pid() AS "BACKEND_PID"
\gset" and you can store it to .psqlrc. But most of the time I am in
customer's environment, and I have the time, possibility to do a
complete setup of .psqlrc. It looks (for me) like a generally useful
feature to be everywhere.

But what do you need the backend PID for in the first place?

Of course, you might want to use it to find your own session in
pg_stat_activity or something like that, but then you're already in a
query and can use pg_backend_pid(). What do you need the backend PID
for outside of such a query?

#18Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#17)
Re: proposal: psql: psql variable BACKEND_PID

po 13. 2. 2023 v 18:06 odesílatel Peter Eisentraut <
peter.eisentraut@enterprisedb.com> napsal:

On 09.02.23 10:11, Pavel Stehule wrote:

first and main (for me) - I can use psql variables tab complete - just
:B<tab> - it is significantly faster
second - I can see all connection related information by \set
third - there is not hook on reconnect in psql - so if you implement
BACKEND_PID by self, you ensure to run query with pg_backend_pid() after
any reconnect or connection change.

It is clean so you can run "select pg_backend_pid() AS "BACKEND_PID"
\gset" and you can store it to .psqlrc. But most of the time I am in
customer's environment, and I have the time, possibility to do a
complete setup of .psqlrc. It looks (for me) like a generally useful
feature to be everywhere.

But what do you need the backend PID for in the first place?

Of course, you might want to use it to find your own session in
pg_stat_activity or something like that, but then you're already in a
query and can use pg_backend_pid(). What do you need the backend PID
for outside of such a query?

In every real use case you can use pg_backend_pid(), but you need to write
a complete name without tab complete, and you need to know so this function
is available.

BACKEND_PID is supported by tab complete, and it is displayed in \set list
and \? variables. Nothing less, nothing more, Custom psql variable can have
some obsolete value.

I can imagine using :BACKEND_PID in \echo command - and it just saves you
one step with its own custom variable.

It is just some more comfort with almost zero cost.

Regards

Pavel

#19Andres Freund
andres@anarazel.de
In reply to: Peter Eisentraut (#17)
Re: proposal: psql: psql variable BACKEND_PID

Hi,

On 2023-02-13 18:06:23 +0100, Peter Eisentraut wrote:

On 09.02.23 10:11, Pavel Stehule wrote:

first and main (for me) - I can use psql variables tab complete - just
:B<tab> - it is significantly faster
second - I can see all connection related information by \set
third - there is not hook on reconnect in psql - so if you implement
BACKEND_PID by self, you ensure to run query with pg_backend_pid() after
any reconnect or connection change.

It is clean so you can run "select pg_backend_pid() AS "BACKEND_PID"
\gset" and you can store it to .psqlrc. But most of the time I am in
customer's environment, and I have the time, possibility to do a
complete setup of .psqlrc. It looks (for me) like a generally useful
feature to be everywhere.

But what do you need the backend PID for in the first place?

For me it's using gdb, pidstat, strace, perf, ...

But for those %p in the PROMPTs is more useful.

Of course, you might want to use it to find your own session in
pg_stat_activity or something like that, but then you're already in a query
and can use pg_backend_pid(). What do you need the backend PID for outside
of such a query?

E.g. I fire of a query, it's slower than I'd like, I want to attach perf. Of
course I can establish a separate connection, query pg_stat_activity there,
and then perf. But that requires manually filtering pg_stat_activity to find
the query.

Greetings,

Andres Freund

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#19)
Re: proposal: psql: psql variable BACKEND_PID

Andres Freund <andres@anarazel.de> writes:

On 2023-02-13 18:06:23 +0100, Peter Eisentraut wrote:

But what do you need the backend PID for in the first place?

For me it's using gdb, pidstat, strace, perf, ...
But for those %p in the PROMPTs is more useful.

Indeed, because ...

E.g. I fire of a query, it's slower than I'd like, I want to attach perf. Of
course I can establish a separate connection, query pg_stat_activity there,
and then perf. But that requires manually filtering pg_stat_activity to find
the query.

... in this case, the problem is that the session is tied up doing the
slow query. You can't run "select pg_backend_pid()", but you can't
extract a psql variable value either. If you had the foresight to
set up a PROMPT, or to collect the PID earlier, you're good. But I'm
still not seeing where a psql variable makes that easier.

I don't buy Pavel's argument that adding Yet Another built-in variable
adds ease of use. I think what it mostly adds is clutter. I realize
that "psql --help=variables | wc" is already 160+ lines, but that
doesn't mean that making it longer and longer is a net improvement.

regards, tom lane

#21Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#20)
#22Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#20)
#23Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#18)
#24Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Pavel Stehule (#22)
#25Pavel Stehule
pavel.stehule@gmail.com
In reply to: Jelte Fennema-Nio (#24)
#26Andrew Dunstan
andrew@dunslane.net
In reply to: Pavel Stehule (#25)
#27Pavel Stehule
pavel.stehule@gmail.com
In reply to: Andrew Dunstan (#26)