Re: [HACKERS] 64-bit queryId?

2017-10-20 Thread Julien Rouhaud
On Fri, Oct 20, 2017 at 3:44 PM, Robert Haas wrote: > On Thu, Oct 19, 2017 at 2:11 AM, Julien Rouhaud wrote: >> I agree, and I'm perfectly fine with adding a comment around pgssHashKey. >> >> PFA a patch to warn about the danger. > > Committed a

Re: [HACKERS] 64-bit queryId?

2017-10-20 Thread Robert Haas
On Thu, Oct 19, 2017 at 2:11 AM, Julien Rouhaud wrote: > I agree, and I'm perfectly fine with adding a comment around pgssHashKey. > > PFA a patch to warn about the danger. Committed a somewhat different version of this - hope you are OK with the result. -- Robert Haas

Re: [HACKERS] 64-bit queryId?

2017-10-19 Thread Julien Rouhaud
On Thu, Oct 19, 2017 at 1:08 AM, Michael Paquier wrote: > On Thu, Oct 19, 2017 at 4:12 AM, Robert Haas wrote: >> On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud wrote: >>> WIth current pgssHashKey definition, there shouldn't

Re: [HACKERS] 64-bit queryId?

2017-10-18 Thread Michael Paquier
On Thu, Oct 19, 2017 at 4:12 AM, Robert Haas wrote: > On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud wrote: >> Sorry for replying so late, but I have a perhaps naive question about >> the hashtable handling with this new version. >> >> IIUC, the shared

Re: [HACKERS] 64-bit queryId?

2017-10-18 Thread Robert Haas
On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud wrote: > Sorry for replying so late, but I have a perhaps naive question about > the hashtable handling with this new version. > > IIUC, the shared hash table is now created with HASH_BLOBS instead of > HASH_FUNCTION, so since

Re: [HACKERS] 64-bit queryId?

2017-10-18 Thread Julien Rouhaud
On Thu, Oct 12, 2017 at 2:46 AM, Michael Paquier wrote: > On Thu, Oct 12, 2017 at 9:05 AM, Robert Haas wrote: >> On Wed, Oct 4, 2017 at 9:00 PM, Michael Paquier >> wrote: >>> v4 looks correct to me. Testing it through

Re: [HACKERS] 64-bit queryId?

2017-10-11 Thread Michael Paquier
On Thu, Oct 12, 2017 at 9:05 AM, Robert Haas wrote: > On Wed, Oct 4, 2017 at 9:00 PM, Michael Paquier > wrote: >> v4 looks correct to me. Testing it through (pgbench and some custom >> queries) I have not spotted issues. If the final decision is

Re: [HACKERS] 64-bit queryId?

2017-10-11 Thread Robert Haas
On Wed, Oct 4, 2017 at 9:00 PM, Michael Paquier wrote: > v4 looks correct to me. Testing it through (pgbench and some custom > queries) I have not spotted issues. If the final decision is to use > 64-bit query IDs, then this patch could be pushed. Cool. Committed,

Re: [HACKERS] 64-bit queryId?

2017-10-04 Thread Michael Paquier
On Thu, Oct 5, 2017 at 4:12 AM, Robert Haas wrote: > On Wed, Oct 4, 2017 at 10:11 AM, Michael Paquier > wrote: >> On Wed, Oct 4, 2017 at 11:04 PM, Robert Haas wrote: >>> Not really; dynahash won't merge two keys just

Re: [HACKERS] 64-bit queryId?

2017-10-04 Thread Robert Haas
On Wed, Oct 4, 2017 at 10:11 AM, Michael Paquier wrote: > On Wed, Oct 4, 2017 at 11:04 PM, Robert Haas wrote: >> Not really; dynahash won't merge two keys just because their hash >> codes come out the same. But you're right; that's probably not

Re: [HACKERS] 64-bit queryId?

2017-10-04 Thread Michael Paquier
On Wed, Oct 4, 2017 at 11:04 PM, Robert Haas wrote: > Not really; dynahash won't merge two keys just because their hash > codes come out the same. But you're right; that's probably not the > best way to do it. TBH, why do we even have pgss_hash_fn? It seems > like using

Re: [HACKERS] 64-bit queryId?

2017-10-04 Thread Robert Haas
On Wed, Oct 4, 2017 at 9:49 AM, Michael Paquier wrote: > I am still on the learning curve with pg_stat_statements... This still > does not look complete to me. pgss_hash_fn only makes use of the last > four bytes of the query ID. What about computing the hash using as >

Re: [HACKERS] 64-bit queryId?

2017-10-04 Thread Michael Paquier
On Wed, Oct 4, 2017 at 9:08 PM, Robert Haas wrote: > On Tue, Oct 3, 2017 at 9:39 PM, Michael Paquier > wrote: >>> I'm sorry, but I don't understand this comment. >> >> I just mean that your patch is correct here. I don't always complain :) > >

Re: [HACKERS] 64-bit queryId?

2017-10-04 Thread Robert Haas
On Tue, Oct 3, 2017 at 9:39 PM, Michael Paquier wrote: >> I'm sorry, but I don't understand this comment. > > I just mean that your patch is correct here. I don't always complain :) Oh, OK. I'm all right with my patch being correct. Here's a new version that

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Michael Paquier
On Wed, Oct 4, 2017 at 10:37 AM, Robert Haas wrote: > On Tue, Oct 3, 2017 at 9:34 PM, Michael Paquier > wrote: >> +/* Write an unsigned integer field (anything written with UINT64_FORMAT) */ >> +#define WRITE_UINT64_FIELD(fldname) \ >> +

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Robert Haas
On Tue, Oct 3, 2017 at 9:34 PM, Michael Paquier wrote: > +/* Write an unsigned integer field (anything written with UINT64_FORMAT) */ > +#define WRITE_UINT64_FIELD(fldname) \ > + appendStringInfo(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \ > +

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Michael Paquier
On Tue, Oct 3, 2017 at 11:55 PM, Robert Haas wrote: > It seems silly to me to throw away a perfectly good bit from the hash > value just because of some risk of minor user confusion. I do like > Michael's suggestion of outputing hexa-like text, but changing the > types of

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Robert Haas
On Tue, Oct 3, 2017 at 12:04 PM, Alexander Korotkov wrote: > BTW, you didn't comment Tom's suggestion about dropping high-order bit which > trades minor user user confusion to minor loss of precision. Oh, I thought I did comment on that. I favor allowing negative IDs

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Alexander Korotkov
On Tue, Oct 3, 2017 at 5:55 PM, Robert Haas wrote: > On Mon, Oct 2, 2017 at 8:07 PM, Alexander Korotkov > wrote: > > +1, > > I see 3 options there: > > 1) Drop high-order bit, as you proposed. > > 2) Allow negative queryIds. > > 3) Implement

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Robert Haas
On Mon, Oct 2, 2017 at 8:07 PM, Alexander Korotkov wrote: > +1, > I see 3 options there: > 1) Drop high-order bit, as you proposed. > 2) Allow negative queryIds. > 3) Implement unsigned 64-type. > > #1 causes minor loss of precision which looks rather insignificant in

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Andres Freund
On 2017-10-03 17:06:20 +0900, Michael Paquier wrote: > On Tue, Oct 3, 2017 at 3:12 PM, Andres Freund wrote: > > On 2017-10-03 03:07:09 +0300, Alexander Korotkov wrote: > >> On Tue, Oct 3, 2017 at 12:32 AM, Tom Lane wrote: > >> +1, > >> I see 3 options

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Michael Paquier
On Tue, Oct 3, 2017 at 3:12 PM, Andres Freund wrote: > On 2017-10-03 03:07:09 +0300, Alexander Korotkov wrote: >> On Tue, Oct 3, 2017 at 12:32 AM, Tom Lane wrote: >> +1, >> I see 3 options there: >> 1) Drop high-order bit, as you proposed. >> 2) Allow

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Vladimir Sitnikov
>OK, so here's a patch. Review appreciated. Please correct typo "Write an unsigned integer field (anythign written with UINT64_FORMAT)". anythign -> anything. Vladimir

Re: [HACKERS] 64-bit queryId?

2017-10-03 Thread Andres Freund
On 2017-10-03 03:07:09 +0300, Alexander Korotkov wrote: > On Tue, Oct 3, 2017 at 12:32 AM, Tom Lane wrote: > > > Peter Geoghegan writes: > > > You need to change the SQL interface as well, although I'm not sure > > > exactly how. The problem is that you are now

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Michael Paquier
On Tue, Oct 3, 2017 at 9:07 AM, Alexander Korotkov wrote: > +1, > I see 3 options there: > 1) Drop high-order bit, as you proposed. > 2) Allow negative queryIds. > 3) Implement unsigned 64-type. > > #1 causes minor loss of precision which looks rather insignificant in

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Alexander Korotkov
On Tue, Oct 3, 2017 at 12:32 AM, Tom Lane wrote: > Peter Geoghegan writes: > > You need to change the SQL interface as well, although I'm not sure > > exactly how. The problem is that you are now passing a uint64 queryId > > to Int64GetDatumFast() within

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Tom Lane
Peter Geoghegan writes: > You need to change the SQL interface as well, although I'm not sure > exactly how. The problem is that you are now passing a uint64 queryId > to Int64GetDatumFast() within pg_stat_statements_internal(). That > worked when queryId was a uint32, because you

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Peter Geoghegan
On Mon, Oct 2, 2017 at 9:10 AM, Robert Haas wrote: > On Mon, Oct 2, 2017 at 11:02 AM, Joshua D. Drake > wrote: >> +1 to both of these as well. > > OK, so here's a patch. Review appreciated. You need to change the SQL interface as well, although

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Gavin Flower
On 03/10/17 04:02, Joshua D. Drake wrote: On 10/01/2017 04:22 PM, Robert Haas wrote: On Sun, Oct 1, 2017 at 3:48 PM, Greg Stark wrote: Well these kinds of monitoring systems tend to be used by operations people who are a lot more practical and a lot less worried about

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Robert Haas
On Mon, Oct 2, 2017 at 11:02 AM, Joshua D. Drake wrote: > +1 to both of these as well. OK, so here's a patch. Review appreciated. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company 64-bit-queryid-v1.patch Description: Binary

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Joshua D. Drake
On 10/01/2017 04:22 PM, Robert Haas wrote: On Sun, Oct 1, 2017 at 3:48 PM, Greg Stark wrote: Well these kinds of monitoring systems tend to be used by operations people who are a lot more practical and a lot less worried about theoretical concerns like that. +1, well said.

Re: [HACKERS] 64-bit queryId?

2017-10-02 Thread Magnus Hagander
On Mon, Oct 2, 2017 at 1:22 AM, Robert Haas wrote: > On Sun, Oct 1, 2017 at 3:48 PM, Greg Stark wrote: > > Well these kinds of monitoring systems tend to be used by operations > > people who are a lot more practical and a lot less worried about > >

Re: [HACKERS] 64-bit queryId?

2017-10-01 Thread Robert Haas
On Sun, Oct 1, 2017 at 3:48 PM, Greg Stark wrote: > Well these kinds of monitoring systems tend to be used by operations > people who are a lot more practical and a lot less worried about > theoretical concerns like that. +1, well said. > In context the point was merely that the

Re: [HACKERS] 64-bit queryId?

2017-10-01 Thread Greg Stark
On 1 October 2017 at 16:40, Tom Lane wrote: > Greg Stark writes: >> Indeed. It's simple enough to export stats to prometheus with queryid >> as the key. Then even if the query ages out of the database stats you >> have graphs and derivative metrics going

Re: [HACKERS] 64-bit queryId?

2017-10-01 Thread Tom Lane
Greg Stark writes: > Indeed. It's simple enough to export stats to prometheus with queryid > as the key. Then even if the query ages out of the database stats you > have graphs and derivative metrics going further back. I'm not really ready to buy into that as a supported

Re: [HACKERS] 64-bit queryId?

2017-10-01 Thread Greg Stark
On 30 September 2017 at 21:03, Alexander Korotkov wrote: > I heard from customers that they periodically dump contents of > pg_stat_statements and then build statistics over long period of time. If > even they leave default pg_stat_statements.max unchanged,

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Andres Freund
On 2017-09-30 18:17:37 -0400, Robert Haas wrote: > On Sat, Sep 30, 2017 at 11:55 AM, Andres Freund wrote: > > On 2017-09-30 11:50:08 -0400, Robert Haas wrote: > >> Well, I think that the fact that pg_stat_statements.max exists at all > >> is something that could be fixed now

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Robert Haas
On Sat, Sep 30, 2017 at 11:55 AM, Andres Freund wrote: > On 2017-09-30 11:50:08 -0400, Robert Haas wrote: >> Well, I think that the fact that pg_stat_statements.max exists at all >> is something that could be fixed now that we have DSA. > > You normally *do* want a limit imo.

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Alexander Korotkov
On Sat, Sep 30, 2017 at 6:39 PM, Peter Geoghegan wrote: > On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas > wrote: > > Assuming, however, that you don't manage to prove all known > > mathematics inconsistent, what one might reasonably hope to do is > > render

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Robert Haas
On Sat, Sep 30, 2017 at 12:03 PM, Tom Lane wrote: > More to the point: with 32-bit IDs, it's apparent that you shouldn't > really rely on them being unique, and should design your usage so that > it will survive collisions. But the point is precisely that we do not do that.

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Andres Freund
On 2017-09-30 12:03:57 -0400, Tom Lane wrote: > Peter Geoghegan writes: > > On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas wrote: > >> Assuming, however, that you don't manage to prove all known > >> mathematics inconsistent, what one might reasonably hope to

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Tom Lane
Peter Geoghegan writes: > On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas wrote: >> Assuming, however, that you don't manage to prove all known >> mathematics inconsistent, what one might reasonably hope to do is >> render collisions remote enough that one need

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Andres Freund
On 2017-09-30 11:50:08 -0400, Robert Haas wrote: > Well, I think that the fact that pg_stat_statements.max exists at all > is something that could be fixed now that we have DSA. You normally *do* want a limit imo. And given that query strings are now stored externally, I don't think there's a

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Robert Haas
On Sat, Sep 30, 2017 at 11:39 AM, Peter Geoghegan wrote: > On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas wrote: >> Assuming, however, that you don't manage to prove all known >> mathematics inconsistent, what one might reasonably hope to do is >> render

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Peter Geoghegan
On Sat, Sep 30, 2017 at 8:39 AM, Peter Geoghegan wrote: > Isn't that already true in the case of queryId? I've never heard any > complaints about collisions. Most people don't change > pg_stat_statements.max, so the probability of a collision is more like > 1%. And, that's the

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Peter Geoghegan
On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas wrote: > Assuming, however, that you don't manage to prove all known > mathematics inconsistent, what one might reasonably hope to do is > render collisions remote enough that one need not worry about them too > much in practice.

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Robert Haas
On Sat, Sep 30, 2017 at 12:34 AM, Tom Lane wrote: > Robert Haas writes: >> How about widening the value to uint64? > > Doesn't really seem like that would guarantee no collisions. Well, no duh. If you come up with a hash function that maps an infinite

Re: [HACKERS] 64-bit queryId?

2017-09-30 Thread Michael Paquier
On Sat, Sep 30, 2017 at 1:34 PM, Tom Lane wrote: > Robert Haas writes: >> How about widening the value to uint64? > > Doesn't really seem like that would guarantee no collisions. This moves the possibility of a 25% collision from 50k queries 3.3

Re: [HACKERS] 64-bit queryId?

2017-09-29 Thread Tom Lane
Robert Haas writes: > How about widening the value to uint64? Doesn't really seem like that would guarantee no collisions. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

[HACKERS] 64-bit queryId?

2017-09-29 Thread Robert Haas
Our Query currently has space for a 32-bit queryId, but that seems reasonably likely to result in collisions: https://en.wikipedia.org/wiki/Birthday_problem#Probability_table If you have as many as 50,000 queries, there's a 25% probability of having at least one collision; that doesn't seem