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 somewhat different version of this - hope you are O
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
EnterpriseDB: http://www.
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 be padding bits,
>>> so it should be safe. But I wonder if adding an e
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 hash table is now created with HASH_BLOBS i
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 sizeof(pgssHashKey) !=
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 (pgbench and some custom
>>> queries) I have not spotted issues. If the final
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 to use
>> 64-bit query IDs, then this patch could b
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, thanks for the review.
--
Rob
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 because their hash
>>> codes come out the same. But you're right; that's proba
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 the
>> best way to do it. TBH, why do we even hav
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 tag_hash would be super
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
> also the first four bytes?
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 :)
>
> Oh, OK. I'm all right with my patch being correct.
>
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 hopefully fixes the things that you
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) \
>> + appendStringInfo(str, " :" CppAsString(fldname) " " UINT6
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, \
> +node->fldname)
> Correct
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 the user-visible output
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
rather than minor loss of pr
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 unsigned 64-type.
> >
> > #1 causes minor loss of prec
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 given
> context.
> #2 might b
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 there:
> >> 1) Drop high-order bit, as you pr
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 negative queryIds.
>> 3) Implement unsigned 6
>OK, so here's a patch. Review appreciated.
Please correct typo "Write an unsigned integer field (anythign written with
UINT64_FORMAT)". anythign -> anything.
Vladimir
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 passing a uint64 queryId
> > > to
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 given
> context.
> #2 might b
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 pg_stat_statements_internal(). That
> > w
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 can easily repre
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 I'm not sure
exactly how. The problem is that you
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
theoretical concerns like
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 data
--
Sent via pgsql-ha
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.
In context the po
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
> > theoretical concerns like that.
>
> +1, well sa
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 default
> pg_sta
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 further back.
>
> I'm not really ready to
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 use-case, because
it imm
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, probability of
> collision would be si
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 that we have DSA.
> >
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. And given that query
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 collisions remote enough that one need
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. pg_stat_statements
"s
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 do is
> >> render collisions remote eno
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 not worry about them too
>> much in pr
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 huge
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 collisions remote enough that one need not w
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 probability of *any*
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.
Isn't that already tru
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 domain onto a finite range while guaranteei
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
billion. Not zero, but safe enough as a minimal ch
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 subscription:
http://www.postgresq
49 matches
Mail list logo
