Обсуждение: [PATCH] Connection time for \conninfo

I've work in the a little patch to add into the \conninfo of psql
command the connection time  against a server backend

Maybe could add after, the precheck to if the connection is alive.
I've take first look to the pqPacketSend, my idea is send a ECHO
packet over to the database connection. If someone has a better idea
it would be great to know.

Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

On Tue, Sep 03, 2019 at 10:13:57PM -0400, Rodrigo Ramírez Norambuena wrote:
> I've work in the a little patch to add into the \conninfo of psql
> command the connection time  against a server backend
>
> Maybe could add after, the precheck to if the connection is alive.
> I've take first look to the pqPacketSend, my idea is send a ECHO
> packet over to the database connection. If someone has a better idea
> it would be great to know.

You can do basically the same thing by looking at
pg_stat_activity.backend_start and compare it with the clock
timestamp.  Doing it at SQL level the way you want has also the
advantage to offer you a modular format output.
--
Michael

On 2019-Sep-04, Michael Paquier wrote:

> On Tue, Sep 03, 2019 at 10:13:57PM -0400, Rodrigo Ramírez Norambuena wrote:
> > I've work in the a little patch to add into the \conninfo of psql
> > command the connection time  against a server backend
> > 
> > Maybe could add after, the precheck to if the connection is alive.
> > I've take first look to the pqPacketSend, my idea is send a ECHO
> > packet over to the database connection. If someone has a better idea
> > it would be great to know.
> 
> You can do basically the same thing by looking at
> pg_stat_activity.backend_start and compare it with the clock
> timestamp.  Doing it at SQL level the way you want has also the
> advantage to offer you a modular format output.

Hmm, couldn't you say the same about the other details that \conninfo
gives?  You can get them from pg_stat_activity or pg_stat_ssl, yet
they're still shown \conninfo for convenience.  Surely we don't want to
remove everything from \conninfo and tell users to query the stat views
instead.

The only thing that seems wrong about this proposal is that the time
format is a bit too verbose.  I would have it do "N days 12:23:34".

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> You can do basically the same thing by looking at
> pg_stat_activity.backend_start and compare it with the clock
> timestamp.  Doing it at SQL level the way you want has also the
> advantage to offer you a modular format output.

Hi Michael,

Thanks you for paying attention, I appreciate this.

What are you say seams little trick to what I propose in this patch
because you'll need filter what is your connection, and if the
connection is through  the connection pooler could be more.

The main goal this is only getting information from the client side
(psql) as frontend.

Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

On Wed, Sep 4, 2019 at 11:04 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> The only thing that seems wrong about this proposal is that the time
> format is a bit too verbose.  I would have it do "N days 12:23:34".
>

Hi Alvaro!,

I attach a second version with this change.

--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

=?UTF-8?Q?Rodrigo_Ram=C3=ADrez_Norambuena?= <decipher.hk@gmail.com> writes:
> On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier <michael@paquier.xyz> wrote:
>> You can do basically the same thing by looking at
>> pg_stat_activity.backend_start and compare it with the clock
>> timestamp.  Doing it at SQL level the way you want has also the
>> advantage to offer you a modular format output.

> What are you say seams little trick to what I propose in this patch
> because you'll need filter what is your connection, and if the
> connection is through  the connection pooler could be more.
> The main goal this is only getting information from the client side
> (psql) as frontend.

A couple of thoughts on this ---

* Michael's right that it's *possible* to get the session start time in
SQL, but having to find one's session's entry in pg_stat_activity is
pretty awkward.  I wonder if we should invent pg_backend_start_time()
analogous to pg_backend_pid().  Most of the other columns in
pg_stat_activity either already have similar functions (e.g. CURRENT_USER)
or don't seem especially useful to be able to retrieve for one's own
session, but this one maybe is.  That's somewhat orthogonal to the
proposed patch but we should probably be considering it in the same
discussion.

* It's not quite clear what the use-case is for displaying this in
\conninfo.  That's easy for humans to look at, but I don't really
understand why a human would care, and it's just about useless for
any programmatic application.

* I wonder why the proposal is to display duration of connection rather
than start time.  I don't especially like the idea that \conninfo
would change every time you look at it, so I'd tend to lean to the
latter, but maybe there are other arguments for one or the other.

* The patch appears to be tracking time measured at the client side,
which is going to be just enough different from the server's idea of
the session start time to be confusing/annoying.  Do we really want
to do it that way?

* There are other definitional questions like what happens when you
reconnect (either due to intentional \connect or momentary connection
loss), or what should happen when you have no connection at all thanks
to a non-recoverable connection loss.  I suppose the patch's code
provides some answers but I'm not sure that it's consistent or well
thought out.  (Commit aef362385 provides some precedent you should look
at in this regard, plus I think this patch has to be rebased over it
anyway.)

BTW, the result type of time(2) is not "int", so this is just wrong:

+    int connected; /* unixtime for connected init time */

This field name is pretty poorly chosen as well; it sounds like
it's a boolean "am I connected" state, and there's certainly no
hint that it's a time value.

            regards, tom lane

On Thu, Sep 5, 2019 at 2:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> =?UTF-8?Q?Rodrigo_Ram=C3=ADrez_Norambuena?= <decipher.hk@gmail.com> writes:
> > On Tue, Sep 3, 2019 at 11:06 PM Michael Paquier <michael@paquier.xyz> wrote:
> >> You can do basically the same thing by looking at
> >> pg_stat_activity.backend_start and compare it with the clock
> >> timestamp.  Doing it at SQL level the way you want has also the
> >> advantage to offer you a modular format output.
>
> > What are you say seams little trick to what I propose in this patch
> > because you'll need filter what is your connection, and if the
> > connection is through  the connection pooler could be more.
> > The main goal this is only getting information from the client side
> > (psql) as frontend.
>
> A couple of thoughts on this ---
> [...]

Hi Tom, about your concerns and feedback I send a new proposal of
patch related with the original idea.

Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

My opinion is that this is not particularly useful and not appropriate 
to piggy-back onto \conninfo.  Connection information including host, 
port, database, user name is a well-established concept in PostgreSQL 
programs and tools and it contains a delimited set of information. 
Knowing what server and what database you are connected to also seems 
kind of important.  Moreover, this is information that is under control 
of the client, so it must be tracked on the client side.

Knowing how long you've been connected on the other hand seems kind of 
fundamentally unimportant.  If we add that, what's to stop us from 
adding other statistics of minor interest such as how many commands 
you've run, how many errors there were, etc.  The connection time is 
already available, and perhaps we should indeed make it a bit easier to 
get, but it doesn't need to be a psql command.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi Rodrigo,

On 2/27/20 4:21 AM, Peter Eisentraut wrote:
> My opinion is that this is not particularly useful and not appropriate 
> to piggy-back onto \conninfo.  Connection information including host, 
> port, database, user name is a well-established concept in PostgreSQL 
> programs and tools and it contains a delimited set of information. 
> Knowing what server and what database you are connected to also seems 
> kind of important.  Moreover, this is information that is under control 
> of the client, so it must be tracked on the client side.
> 
> Knowing how long you've been connected on the other hand seems kind of 
> fundamentally unimportant.  If we add that, what's to stop us from 
> adding other statistics of minor interest such as how many commands 
> you've run, how many errors there were, etc.  The connection time is 
> already available, and perhaps we should indeed make it a bit easier to 
> get, but it doesn't need to be a psql command.

The consensus from the committers (with the exception of Álvaro) seems 
to be that this is not a feature we want. FWIW, I agree with the majority.

I'll mark this as rejected on MAR-16 unless Álvaro makes an argument for 
committing it.

Regards,
-- 
-David
david@pgmasters.net

Greetings,

* David Steele (david@pgmasters.net) wrote:
> On 2/27/20 4:21 AM, Peter Eisentraut wrote:
> >My opinion is that this is not particularly useful and not appropriate to
> >piggy-back onto \conninfo.  Connection information including host, port,
> >database, user name is a well-established concept in PostgreSQL programs
> >and tools and it contains a delimited set of information. Knowing what
> >server and what database you are connected to also seems kind of
> >important.  Moreover, this is information that is under control of the
> >client, so it must be tracked on the client side.

I have to say that I disagree.  Wishing to know when you connected to a
server is entirely reasonable and it's also rather clearly under control
of the client (though I don't entirely understand that argument in the
first place).

> >Knowing how long you've been connected on the other hand seems kind of
> >fundamentally unimportant.  If we add that, what's to stop us from adding
> >other statistics of minor interest such as how many commands you've run,
> >how many errors there were, etc.  The connection time is already
> >available, and perhaps we should indeed make it a bit easier to get, but
> >it doesn't need to be a psql command.

Of 'minor' interest, or not, if it's something you'd like to know then
it's pretty handy if there's a way to get that info.  This also isn't
inventing a new psql command but rather adding on to one that's clearly
already a 'creature comfort' command as everything included could be
gotten at in other ways.

As for the comparison to other things of minor interest, we do give
users a way to get things like line number (%l in PROMPT), so including
something akin to that certainly doesn't seem out of the question.
Having a '%T' or something in the prompt for connected-at-time seems
like it could certainly be useful too.

Going a bit farther, I wonder if conninfo could be configurable as a
Prompt-like string, that seems like it'd be pretty neat.

Anyway, I don't anticipate having time to do anything with this patch
but I disagree that this is a "we don't want it" kind of thing, rather
we maybe want it, since someone cared enough to write a patch, but the
patch needs work and maybe we want it to look a bit different and be
better defined.

Thanks,

Stephen

On 2020-03-10 18:38, Stephen Frost wrote:
>> On 2/27/20 4:21 AM, Peter Eisentraut wrote:
>>> My opinion is that this is not particularly useful and not appropriate to
>>> piggy-back onto \conninfo.  Connection information including host, port,
>>> database, user name is a well-established concept in PostgreSQL programs
>>> and tools and it contains a delimited set of information. Knowing what
>>> server and what database you are connected to also seems kind of
>>> important.  Moreover, this is information that is under control of the
>>> client, so it must be tracked on the client side.
> 
> I have to say that I disagree.  Wishing to know when you connected to a
> server is entirely reasonable and it's also rather clearly under control
> of the client (though I don't entirely understand that argument in the
> first place).

The argument is that the server already knows when the client connected 
and that information is already available.  So there is no reason for 
the client to also track and display that information, other than 
perhaps convenience.  But the server does not, in general, know what 
host and port the client connected to (because of proxying and other 
network stuff), so this is something that the client must record itself.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Stephen Frost <sfrost@snowman.net> writes:
> Anyway, I don't anticipate having time to do anything with this patch
> but I disagree that this is a "we don't want it" kind of thing, rather
> we maybe want it, since someone cared enough to write a patch, but the
> patch needs work and maybe we want it to look a bit different and be
> better defined.

I think Peter's primary argument was that this doesn't belong in
\conninfo, which is about reporting the parameters required to
establish the connection.  We have kind of broken that already by
cramming SSL and GSS encryption info into the results, but that
doesn't mean it should become a kitchen-sink listing of anything
anybody says they'd like to know.

Anyway, I think your point is that maybe this should be RWF
not Rejected, and I agree with that.

(I had not looked at the last version of the patch, but now that
I have, I still don't like the fact that it has the client tracking
session start time separately from what the server does.  The small
discrepancy that introduces is going to confuse somebody.  I see
that there's no documentation update either.)

            regards, tom lane

Greetings,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> On 2020-03-10 18:38, Stephen Frost wrote:
> >>On 2/27/20 4:21 AM, Peter Eisentraut wrote:
> >>>My opinion is that this is not particularly useful and not appropriate to
> >>>piggy-back onto \conninfo.  Connection information including host, port,
> >>>database, user name is a well-established concept in PostgreSQL programs
> >>>and tools and it contains a delimited set of information. Knowing what
> >>>server and what database you are connected to also seems kind of
> >>>important.  Moreover, this is information that is under control of the
> >>>client, so it must be tracked on the client side.
> >
> >I have to say that I disagree.  Wishing to know when you connected to a
> >server is entirely reasonable and it's also rather clearly under control
> >of the client (though I don't entirely understand that argument in the
> >first place).
>
> The argument is that the server already knows when the client connected and
> that information is already available.  So there is no reason for the client
> to also track and display that information, other than perhaps convenience.

Yes, it'd be for convenience.

> But the server does not, in general, know what host and port the client
> connected to (because of proxying and other network stuff), so this is
> something that the client must record itself.

I agree that there are some things the client has to track and not ask
the server for, because the server's answers could be different, but I
don't agree that conninfo should only ever include that info (and I
seriously doubt that was the original idea, considering 'database' is
included at least as far back as 1999..).

As a side-note, we should probably update our documentation for psql,
since the description of things under Prompting tends to presume that
there isn't anything between the client and the server (for example,
%> says "The port number at which the database server is listening.",
but it's really "The port number used to connect to the database
server." or something along those lines).

Thanks,

Stephen

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > Anyway, I don't anticipate having time to do anything with this patch
> > but I disagree that this is a "we don't want it" kind of thing, rather
> > we maybe want it, since someone cared enough to write a patch, but the
> > patch needs work and maybe we want it to look a bit different and be
> > better defined.
>
> I think Peter's primary argument was that this doesn't belong in
> \conninfo, which is about reporting the parameters required to
> establish the connection.  We have kind of broken that already by
> cramming SSL and GSS encryption info into the results, but that
> doesn't mean it should become a kitchen-sink listing of anything
> anybody says they'd like to know.

I could certainly agree with wanting to have a psql command that's "give
me what I need to connect", but that idea and what conninfo actually
returns are pretty distant from each other.  For one thing, if I wanted
that from psql, I'd sure hope to get back something that I could
directly use when starting up a new psql session.

> Anyway, I think your point is that maybe this should be RWF
> not Rejected, and I agree with that.

Ok.

> (I had not looked at the last version of the patch, but now that
> I have, I still don't like the fact that it has the client tracking
> session start time separately from what the server does.  The small
> discrepancy that introduces is going to confuse somebody.  I see
> that there's no documentation update either.)

This worries me about as much as I worry that someone's going to be
confused by explain-analyze output vs. \timing.  Yes, it happens, and
actually pretty often, but I wouldn't change how it works because
they're two different things, not to mention that if I'm going to be
impacted by the time being off on one of the systems, I'd at least like
to know when my client thought it connected relative to the clock on my
client.

THanks,

Stephen

Hi There!

I forgot about that ... It passed a little time from my new pushed
changes. So go ahead :)

On Tue, Mar 10, 2020 at 3:15 PM Stephen Frost <sfrost@snowman.net> wrote:
>
> Greetings,
>
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> > Stephen Frost <sfrost@snowman.net> writes:
> > > Anyway, I don't anticipate having time to do anything with this patch
> > > but I disagree that this is a "we don't want it" kind of thing, rather
> > > we maybe want it, since someone cared enough to write a patch, but the
> > > patch needs work and maybe we want it to look a bit different and be
> > > better defined.
> >
> > I think Peter's primary argument was that this doesn't belong in
> > \conninfo, which is about reporting the parameters required to
> > establish the connection.  We have kind of broken that already by
> > cramming SSL and GSS encryption info into the results, but that
> > doesn't mean it should become a kitchen-sink listing of anything
> > anybody says they'd like to know.
>
> I could certainly agree with wanting to have a psql command that's "give
> me what I need to connect", but that idea and what conninfo actually
> returns are pretty distant from each other.  For one thing, if I wanted
> that from psql, I'd sure hope to get back something that I could
> directly use when starting up a new psql session.
>
> > Anyway, I think your point is that maybe this should be RWF
> > not Rejected, and I agree with that.
>
> Ok.
>
> > (I had not looked at the last version of the patch, but now that
> > I have, I still don't like the fact that it has the client tracking
> > session start time separately from what the server does.  The small
> > discrepancy that introduces is going to confuse somebody.  I see
> > that there's no documentation update either.)
>
> This worries me about as much as I worry that someone's going to be
> confused by explain-analyze output vs. \timing.  Yes, it happens, and
> actually pretty often, but I wouldn't change how it works because
> they're two different things, not to mention that if I'm going to be
> impacted by the time being off on one of the systems, I'd at least like
> to know when my client thought it connected relative to the clock on my
> client.

So if the path it set as RWF could push a extra work in there but in
main point what be the address about that.


Thanks to all for you feedback.
Regards!
--
Rodrigo Ramírez Norambuena
http://www.rodrigoramirez.com/

On 3/14/20 3:16 PM, Rodrigo Ramírez Norambuena wrote:
> 
> I forgot about that ... It passed a little time from my new pushed
> changes. So go ahead :)

This patch has been returned with feedback. Please feel free to resubmit 
in a future CF when you believe the feedback has been addressed.

Regards,
-- 
-David
david@pgmasters.net

On 3/16/20 10:59 AM, Juan José Santamaría Flecha wrote:
> On Mon, Mar 16, 2020 at 1:24 PM David Steele <david@pgmasters.net 
> <mailto:david@pgmasters.net>> wrote:
> 
>     On 3/14/20 3:16 PM, Rodrigo Ramírez Norambuena wrote:
>      >
>      > I forgot about that ... It passed a little time from my new pushed
>      > changes. So go ahead :)
> 
>     This patch has been returned with feedback. Please feel free to
>     resubmit
>     in a future CF when you believe the feedback has been addressed.
> 
> 
> There is a duplicate entry as:

Thanks.  I've marked in as withdrawn but in the future you can feel free 
to do so yourself.

Regards,
-- 
-David
david@pgmasters.net