Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Andres Freund
Hi, On 2021-09-16 16:38:04 +0200, Magnus Hagander wrote: > On Wed, Sep 15, 2021 at 10:48 PM Andres Freund wrote: > > > > On 2021-09-15 10:47:33 -0400, Andrew Dunstan wrote: > > > this is an open item for release 14. Is someone going to commit? > > > > Will do. Although I do wish the original

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Magnus Hagander
On Wed, Sep 15, 2021 at 10:48 PM Andres Freund wrote: > > On 2021-09-15 10:47:33 -0400, Andrew Dunstan wrote: > > this is an open item for release 14. Is someone going to commit? > > Will do. Although I do wish the original committer would have chimed in at > some point... Crap. My apologies for

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Laurenz Albe
On Thu, 2021-09-16 at 02:22 -0700, Andres Freund wrote: > I pushed this. The only substantive change I made is that I moved the > MyBackendType == B_BACKEND check into a new pgstat_should_report_connstat(), > and called that from pgstat_report_connect() and > pgstat_report_disconnect(). Otherwise

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Andres Freund
Hi, On 2021-09-08 06:11:56 +0200, Laurenz Albe wrote: > I have gone over your patch and made the following changes: > > - cache the last report time in a static variable pgLastSessionReportTime > - add a comment to explain why we only track normal backends > - pgindent > - an attempt at a commit

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-15 Thread Michael Paquier
On Wed, Sep 15, 2021 at 01:48:09PM -0700, Andres Freund wrote: > Will do. Although I do wish the original committer would have chimed in at > some point... Thanks, Andres. -- Michael signature.asc Description: PGP signature

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-15 Thread Andres Freund
On 2021-09-15 10:47:33 -0400, Andrew Dunstan wrote: > this is an open item for release 14. Is someone going to commit? Will do. Although I do wish the original committer would have chimed in at some point...

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-15 Thread Andrew Dunstan
On 9/8/21 12:11 AM, Laurenz Albe wrote: > On Fri, 2021-09-03 at 17:04 -0700, Andres Freund wrote: >> Here's how I think that would look like. While writing up this draft, I found >> two more issues: >> >> - On windows / 32 bit systems, the session time would overflow if idle for >>   longer than

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-07 Thread Laurenz Albe
On Fri, 2021-09-03 at 17:04 -0700, Andres Freund wrote: > Here's how I think that would look like. While writing up this draft, I found > two more issues: > > - On windows / 32 bit systems, the session time would overflow if idle for >   longer than ~4300s. long is only 32 bit. Easy to fix

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-06 Thread Laurenz Albe
On Mon, 2021-09-06 at 00:22 -0700, Andres Freund wrote: > On 2021-09-06 09:12:58 +0200, Laurenz Albe wrote: > > Reading your patch, I am still confused about the following: > > There are potentially several calls to "pgstat_send_tabstat" in > > "pgstat_report_stat". > > It seems to me that if it

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-06 Thread Andres Freund
Hi, On 2021-09-06 09:12:58 +0200, Laurenz Albe wrote: > Reading your patch, I am still confused about the following: > There are potentially several calls to "pgstat_send_tabstat" in > "pgstat_report_stat". > It seems to me that if it were called more than once, session statistics would > be

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-06 Thread Laurenz Albe
On Fri, 2021-09-03 at 17:04 -0700, Andres Freund wrote: > Hi, > > On 2021-08-31 21:56:50 -0700, Andres Freund wrote: > > On 2021-08-27 13:57:45 +0900, Michael Paquier wrote: > > > On Wed, Aug 25, 2021 at 01:20:03AM -0700, Andres Freund wrote: > > > > On 2021-08-25 12:51:58 +0900, Michael Paquier

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-03 Thread Andres Freund
Hi, On 2021-08-31 21:56:50 -0700, Andres Freund wrote: > On 2021-08-27 13:57:45 +0900, Michael Paquier wrote: > > On Wed, Aug 25, 2021 at 01:20:03AM -0700, Andres Freund wrote: > > > On 2021-08-25 12:51:58 +0900, Michael Paquier wrote: > > > As I said before, this ship has long sailed: > > > > >

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-02 Thread Laurenz Albe
On Wed, 2021-09-01 at 10:56 +0200, Laurenz Albe wrote: > On Tue, 2021-08-31 at 21:16 -0700, Andres Freund wrote: > > On 2021-09-01 05:39:14 +0200, Laurenz Albe wrote: > > > On Tue, 2021-08-31 at 18:55 -0700, Andres Freund wrote: > > > > > > On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-01 Thread Laurenz Albe
On Tue, 2021-08-31 at 21:16 -0700, Andres Freund wrote: > On 2021-09-01 05:39:14 +0200, Laurenz Albe wrote: > > On Tue, 2021-08-31 at 18:55 -0700, Andres Freund wrote: > > > > > On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe wrote:In the > > > > > view of that, how about doubling

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-31 Thread Andres Freund
On 2021-08-27 13:57:45 +0900, Michael Paquier wrote: > On Wed, Aug 25, 2021 at 01:20:03AM -0700, Andres Freund wrote: > > On 2021-08-25 12:51:58 +0900, Michael Paquier wrote: > > As I said before, this ship has long sailed: > > > > typedef struct PgStat_MsgTabstat > > { > > PgStat_MsgHdr

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-31 Thread Andres Freund
On 2021-09-01 05:39:14 +0200, Laurenz Albe wrote: > On Tue, 2021-08-31 at 18:55 -0700, Andres Freund wrote: > > > > On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe wrote:In the > > > > view of that, how about doubling PGSTAT_STAT_INTERVAL to 1000 > > > > > > > milliseconds?  That would

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-31 Thread Laurenz Albe
On Wed, 2021-09-01 at 10:33 +0900, Michael Paquier wrote: > On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe wrote: > > In the view of that, how about doubling PGSTAT_STAT_INTERVAL to 1000 > > milliseconds? > > Perhaps we could do that.  Now, increasing an interval for the sake of >

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-31 Thread Laurenz Albe
On Tue, 2021-08-31 at 18:55 -0700, Andres Freund wrote: > > > On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe wrote:In the view > > > of that, how about doubling PGSTAT_STAT_INTERVAL to 1000 > > > > > milliseconds?  That would mean slightly less up-to-date statistics, but > > > I doubt

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-31 Thread Andres Freund
Hi, On August 31, 2021 6:33:15 PM PDT, Michael Paquier wrote: >On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe wrote: >> In the view of that, how about doubling PGSTAT_STAT_INTERVAL to 1000 >> milliseconds? That would mean slightly less up-to-date statistics, but >> I doubt that that

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-31 Thread Michael Paquier
On Tue, Aug 31, 2021 at 04:55:35AM +0200, Laurenz Albe wrote: > In the view of that, how about doubling PGSTAT_STAT_INTERVAL to 1000 > milliseconds? That would mean slightly less up-to-date statistics, but > I doubt that that will be a problem. And it should even out the increase > in statistics

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-30 Thread Laurenz Albe
On Fri, 2021-08-27 at 13:57 +0900, Michael Paquier wrote: > > I think in that case we'd have to do the bigger redesign and move "live" > > connection stats to backend_status.c... > > Hmm.  A redesign is not really an option for 14 at this stage.  And I > am not really comfortable with the latest

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-26 Thread Michael Paquier
On Wed, Aug 25, 2021 at 01:20:03AM -0700, Andres Freund wrote: > On 2021-08-25 12:51:58 +0900, Michael Paquier wrote: > As I said before, this ship has long sailed: > > typedef struct PgStat_MsgTabstat > { > PgStat_MsgHdr m_hdr; > Oid m_databaseid; > int

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-25 Thread Andres Freund
Hi, On 2021-08-25 12:51:58 +0900, Michael Paquier wrote: > I was looking at this WIP patch, and plugging in the connection > statistics to the table-access statistics looks like the wrong > abstraction to me. I find much cleaner the approach of HEAD to use a > separate API to report this

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-25 Thread Andres Freund
Hi, On 2021-08-20 14:27:20 -0500, Justin Pryzby wrote: > On Tue, Aug 17, 2021 at 02:14:20AM -0700, Andres Freund wrote: > > Doubling the number of UDP messages in common workloads seems also > > problematic > > enough that it should be addressed for 14. It increases the likelihood of > >

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-24 Thread Kyotaro Horiguchi
At Wed, 25 Aug 2021 13:21:52 +0900 (JST), Kyotaro Horiguchi wrote in > At Wed, 25 Aug 2021 12:51:58 +0900, Michael Paquier > wrote in > > - Throttle the frequency where the connection stat packages are sent, > > as of [2]. > > > > Magnus, this open item is assigned to you as the committer

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-24 Thread Kyotaro Horiguchi
At Wed, 25 Aug 2021 12:51:58 +0900, Michael Paquier wrote in > On Wed, Aug 25, 2021 at 10:12:41AM +0900, Kyotaro Horiguchi wrote: > > Yes, it can be called two or more times for a call to > > pgstat_report_stat, but the patch causes only the first of them convey > > effective connection stats

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-24 Thread Michael Paquier
On Wed, Aug 25, 2021 at 10:12:41AM +0900, Kyotaro Horiguchi wrote: > Yes, it can be called two or more times for a call to > pgstat_report_stat, but the patch causes only the first of them convey > effective connection stats numbers. This is the same way as how > transaction stats are treated. If

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-24 Thread Kyotaro Horiguchi
At Tue, 24 Aug 2021 12:34:25 +0200, Laurenz Albe wrote in > On Tue, 2021-08-24 at 15:12 +0900, Kyotaro Horiguchi wrote: > > > > - remove redundant gettimeofday(). > > - avoid sending dedicate UCP packet for connection stats. > > Thank you. > > Perhaps I misread that, but doesn't that mean

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-24 Thread Laurenz Albe
On Tue, 2021-08-24 at 15:12 +0900, Kyotaro Horiguchi wrote: > At Wed, 18 Aug 2021 05:16:38 +0200, Laurenz Albe > wrote in > > On Tue, 2021-08-17 at 02:14 -0700, Andres Freund wrote: > > > > > I'm also not all that happy with sending yet another UDP packet for > > > > > this. > > > > > > > >

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-24 Thread Kyotaro Horiguchi
At Wed, 18 Aug 2021 05:16:38 +0200, Laurenz Albe wrote in > On Tue, 2021-08-17 at 02:14 -0700, Andres Freund wrote: > > On 2021-08-17 10:44:51 +0200, Laurenz Albe wrote: > > > On Sun, 2021-08-01 at 13:55 -0700, Andres Freund wrote: > > > > We maintain last_report as

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-20 Thread Justin Pryzby
On Tue, Aug 17, 2021 at 02:14:20AM -0700, Andres Freund wrote: > Doubling the number of UDP messages in common workloads seems also problematic > enough that it should be addressed for 14. It increases the likelihood of > dropping stats messages on busy systems, which can have downstream impacts.

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-17 Thread Laurenz Albe
On Tue, 2021-08-17 at 02:14 -0700, Andres Freund wrote: > On 2021-08-17 10:44:51 +0200, Laurenz Albe wrote: > > On Sun, 2021-08-01 at 13:55 -0700, Andres Freund wrote: > > > We maintain last_report as GetCurrentTransactionStopTimestamp(), but then > > > use > > > a separate timestamp in

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-17 Thread Andres Freund
Hi, On 2021-08-17 10:44:51 +0200, Laurenz Albe wrote: > On Sun, 2021-08-01 at 13:55 -0700, Andres Freund wrote: > > We maintain last_report as GetCurrentTransactionStopTimestamp(), but then > > use > > a separate timestamp in pgstat_send_connstats() to compute the difference > > from > >

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-17 Thread Laurenz Albe
On Sun, 2021-08-01 at 13:55 -0700, Andres Freund wrote: > Since > > commit 960869da0803427d14335bba24393f414b476e2c > Author: Magnus Hagander > Date: 2021-01-17 13:34:09 +0100 > > Add pg_stat_database counters for sessions and session time > > pgstat_report_stat() does another timestamp

pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-08-01 Thread Andres Freund
Hi, Since commit 960869da0803427d14335bba24393f414b476e2c Author: Magnus Hagander Date: 2021-01-17 13:34:09 +0100 Add pg_stat_database counters for sessions and session time pgstat_report_stat() does another timestamp computation via pgstat_send_connstats(), despite typically having