Content-Type: multipart/mixed; boundary="------------XffYrhKbuz6vw56dfTH6ImkU"
Message-ID: <b589ba2e-606e-4ade-8f42-2096458be8ec@tantorlabs.com>
Date: Thu, 21 Nov 2024 12:17:22 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Sample rate added to pg_stat_statements
To: Michael Paquier <michael@paquier.xyz>,
 Greg Sabino Mullane <htamfids@gmail.com>
Cc: "Andrey M. Borodin" <x4mmm@yandex-team.ru>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <fe99e0ca-e564-480e-b865-5f0cee30bc60@tantorlabs.com>
 <c7ce658a-e0eb-4c5f-92a8-511314d89bbb@tantorlabs.com>
 <6707FCA9-FD1A-4609-A1A6-142456C14E0C@yandex-team.ru>
 <CAKAnmmLD8C-Y6CVF74Ccup8h9YpzBYzSvWWRG_Qf8+5YEMh4-g@mail.gmail.com>
 <Zz0MFPq1s4WFxxpL@paquier.xyz>
Content-Language: en-US
From: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
In-Reply-To: <Zz0MFPq1s4WFxxpL@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/b589ba2e-606e-4ade-8f42-2096458be8ec%40tantorlabs.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------XffYrhKbuz6vw56dfTH6ImkU
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 20.11.2024 01:07, Michael Paquier wrote:
> On Tue, Nov 19, 2024 at 09:39:21AM -0500, Greg Sabino Mullane wrote:
>> Oh, and a +1 in general to the patch, OP, although it would also be nice to
>> start finding the bottlenecks that cause such performance issues.
> FWIW, I'm not eager to integrate this proposal without looking at this
> exact argument in depth.
>
> One piece of it would be to see how much of such "bottlenecks" we
> would be able to get rid of by integrating pg_stat_statements into
> the central pgstats with the custom APIs, without pushing the module
> into core.  This means that we would combine the existing hash of pgss
> to shrink to 8 bytes for objid rather than 13 bytes now as the current
> code relies on (toplevel, userid, queryid) for the entry lookup (entry
> removal is sniped with these three values as well, or dshash seq
> scans).  The odds of conflicts still still play in our favor even if
> we have a few million entries, or even ten times that.
>
> This would also get rid of the pgss text file for the queries, which
> is a source of one of the bottlenecks, as we could just store query
> strings upper-bounded based on a postmaster GUC to control the size of
> the entries in the pgstats dshash.  More normalization for IN and ANY
> clauses would also help a not here, these are a cause of a lot of
> bloat.
>
> This integration is not something I will be able to work on for the
> PG18 dev cycle as I'm in full review/commit mode for the rest of this
> release, but I got some plans for it in PG19 except if somebody beats
> me to it.
> --
> Michael


I agree. Your proposal can indeed improve performance. Currently, I am 
working on these changes and will validate them with benchmarks. Once I 
have concrete results, I will open new threads to facilitate further 
discussion.

However, in my opinion, the suggested improvements are not enough, and 
sampling is essential.

1. I agree with Greg that pgss is widely used. It's quite odd that 
sampling exists in 'auto_explain' but not in pgss.

2. If performance issues arise even after these improvements and it 
turns out that pgss is the cause, the only painless solution without 
restarting the instance is sampling. The current pgss's parameters are 
not optimal for achieving this.

BTW, I forgot to include a case of nested statements. Either all will be 
tracked or none. I attached new version of patch.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.

--------------XffYrhKbuz6vw56dfTH6ImkU
Content-Type: text/x-patch; charset=UTF-8;
 name="v4-0001-Allow-setting-sample-ratio-for-pg_stat_statements.patch"
Content-Disposition: attachment;
 filename*0="v4-0001-Allow-setting-sample-ratio-for-pg_stat_statements.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSA2OGY1NDUxMDE5YjI2MWJmMDNhMjIyZjVhMDVhYzU3Y2QwZmI5YTI0IE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBJbGlhIEV2ZG9raW1vdiA8aWx5YS5ldmRva2ltb3ZA
dGFudG9ybGFicy5jb20+CkRhdGU6IFRodSwgMjEgTm92IDIwMjQgMTE6MjQ6MDMgKzAzMDAK
U3ViamVjdDogW1BBVENIXSBBbGxvdyBzZXR0aW5nIHNhbXBsZSByYXRpbyBmb3IgcGdfc3Rh
dF9zdGF0ZW1lbnRzCgpOZXcgY29uZmlndXJhdGlvbiBwYXJhbWV0ZXIgcGdfc3RhdF9zdGF0
ZW1lbnRzLnNhbXBsZV9yYXRpbyBtYWtlcyBpdApwb3NzaWJsZSB0byB0cmFjayBqdXN0IGEg
ZnJhY3Rpb24gb2YgdGhlIHF1ZXJpZXMgbWVldGluZyB0aGUgY29uZmlndXJlZAp0aHJlc2hv
bGQsIHRvIHJlZHVjZSB0aGUgYW1vdW50IG9mIHRyYWNraW5nLgotLS0KIC4uLi9wZ19zdGF0
X3N0YXRlbWVudHMvcGdfc3RhdF9zdGF0ZW1lbnRzLmMgICB8IDIzICsrKysrKysrKysrKysr
KysrKy0KIGRvYy9zcmMvc2dtbC9wZ3N0YXRzdGF0ZW1lbnRzLnNnbWwgICAgICAgICAgICB8
IDE4ICsrKysrKysrKysrKysrKwogMiBmaWxlcyBjaGFuZ2VkLCA0MCBpbnNlcnRpb25zKCsp
LCAxIGRlbGV0aW9uKC0pCgpkaWZmIC0tZ2l0IGEvY29udHJpYi9wZ19zdGF0X3N0YXRlbWVu
dHMvcGdfc3RhdF9zdGF0ZW1lbnRzLmMgYi9jb250cmliL3BnX3N0YXRfc3RhdGVtZW50cy9w
Z19zdGF0X3N0YXRlbWVudHMuYwppbmRleCA0OWM2NTdiM2UwLi5kMDZlMGQ4YTQ0IDEwMDY0
NAotLS0gYS9jb250cmliL3BnX3N0YXRfc3RhdGVtZW50cy9wZ19zdGF0X3N0YXRlbWVudHMu
YworKysgYi9jb250cmliL3BnX3N0YXRfc3RhdGVtZW50cy9wZ19zdGF0X3N0YXRlbWVudHMu
YwpAQCAtNDksNiArNDksNyBAQAogCiAjaW5jbHVkZSAiYWNjZXNzL3BhcmFsbGVsLmgiCiAj
aW5jbHVkZSAiY2F0YWxvZy9wZ19hdXRoaWQuaCIKKyNpbmNsdWRlICJjb21tb24vcGdfcHJu
Zy5oIgogI2luY2x1ZGUgImNvbW1vbi9pbnQuaCIKICNpbmNsdWRlICJleGVjdXRvci9pbnN0
cnVtZW50LmgiCiAjaW5jbHVkZSAiZnVuY2FwaS5oIgpAQCAtMjk0LDEyICsyOTUsMTYgQEAg
c3RhdGljIGJvb2wgcGdzc190cmFja191dGlsaXR5ID0gdHJ1ZTsJLyogd2hldGhlciB0byB0
cmFjayB1dGlsaXR5IGNvbW1hbmRzICovCiBzdGF0aWMgYm9vbCBwZ3NzX3RyYWNrX3BsYW5u
aW5nID0gZmFsc2U7CS8qIHdoZXRoZXIgdG8gdHJhY2sgcGxhbm5pbmcKIAkJCQkJCQkJCQkJ
ICogZHVyYXRpb24gKi8KIHN0YXRpYyBib29sIHBnc3Nfc2F2ZSA9IHRydWU7CS8qIHdoZXRo
ZXIgdG8gc2F2ZSBzdGF0cyBhY3Jvc3Mgc2h1dGRvd24gKi8KK3N0YXRpYyBkb3VibGUgcGdz
c19zYW1wbGVfcmF0ZSA9IDE7CiAKKy8qIElzIHRoZSBjdXJyZW50IHRvcC1sZXZlbCBxdWVy
eSB0byBiZSBzYW1wbGVkPyAqLworc3RhdGljIGJvb2wgY3VycmVudF9xdWVyeV9zYW1wbGVk
ID0gZmFsc2U7CiAKICNkZWZpbmUgcGdzc19lbmFibGVkKGxldmVsKSBcCiAJKCFJc1BhcmFs
bGVsV29ya2VyKCkgJiYgXAogCShwZ3NzX3RyYWNrID09IFBHU1NfVFJBQ0tfQUxMIHx8IFwK
LQkocGdzc190cmFjayA9PSBQR1NTX1RSQUNLX1RPUCAmJiAobGV2ZWwpID09IDApKSkKKwko
cGdzc190cmFjayA9PSBQR1NTX1RSQUNLX1RPUCAmJiAobGV2ZWwpID09IDApKSAmJiBcCisJ
Y3VycmVudF9xdWVyeV9zYW1wbGVkKQogCiAjZGVmaW5lIHJlY29yZF9nY19xdGV4dHMoKSBc
CiAJZG8geyBcCkBAIC00MTQsNiArNDE5LDE5IEBAIF9QR19pbml0KHZvaWQpCiAJCQkJCQkJ
TlVMTCwKIAkJCQkJCQlOVUxMKTsKIAorCURlZmluZUN1c3RvbVJlYWxWYXJpYWJsZSgicGdf
c3RhdF9zdGF0ZW1lbnRzLnNhbXBsZV9yYXRlIiwKKwkJCQkJCQkgIkZyYWN0aW9uIG9mIHF1
ZXJpZXMgdG8gcHJvY2Vzcy4iLAorCQkJCQkJCSBOVUxMLAorCQkJCQkJCSAmcGdzc19zYW1w
bGVfcmF0ZSwKKwkJCQkJCQkgMS4wLAorCQkJCQkJCSAwLjAsCisJCQkJCQkJIDEuMCwKKwkJ
CQkJCQkgUEdDX1NVU0VULAorCQkJCQkJCSAwLAorCQkJCQkJCSBOVUxMLAorCQkJCQkJCSBO
VUxMLAorCQkJCQkJCSBOVUxMKTsKKwogCURlZmluZUN1c3RvbUVudW1WYXJpYWJsZSgicGdf
c3RhdF9zdGF0ZW1lbnRzLnRyYWNrIiwKIAkJCQkJCQkgIlNlbGVjdHMgd2hpY2ggc3RhdGVt
ZW50cyBhcmUgdHJhY2tlZCBieSBwZ19zdGF0X3N0YXRlbWVudHMuIiwKIAkJCQkJCQkgTlVM
TCwKQEAgLTgzNSw2ICs4NTMsOSBAQCBwZ3NzX3Bvc3RfcGFyc2VfYW5hbHl6ZShQYXJzZVN0
YXRlICpwc3RhdGUsIFF1ZXJ5ICpxdWVyeSwgSnVtYmxlU3RhdGUgKmpzdGF0ZSkKIAlpZiAo
cHJldl9wb3N0X3BhcnNlX2FuYWx5emVfaG9vaykKIAkJcHJldl9wb3N0X3BhcnNlX2FuYWx5
emVfaG9vayhwc3RhdGUsIHF1ZXJ5LCBqc3RhdGUpOwogCisJaWYgKG5lc3RpbmdfbGV2ZWwg
PT0gMCkKKwkJY3VycmVudF9xdWVyeV9zYW1wbGVkID0gKHBnX3BybmdfZG91YmxlKCZwZ19n
bG9iYWxfcHJuZ19zdGF0ZSkgPCBwZ3NzX3NhbXBsZV9yYXRlKTsKKwogCS8qIFNhZmV0eSBj
aGVjay4uLiAqLwogCWlmICghcGdzcyB8fCAhcGdzc19oYXNoIHx8ICFwZ3NzX2VuYWJsZWQo
bmVzdGluZ19sZXZlbCkpCiAJCXJldHVybjsKZGlmZiAtLWdpdCBhL2RvYy9zcmMvc2dtbC9w
Z3N0YXRzdGF0ZW1lbnRzLnNnbWwgYi9kb2Mvc3JjL3NnbWwvcGdzdGF0c3RhdGVtZW50cy5z
Z21sCmluZGV4IDUwMWI0NjhlOWEuLmQwNjM0OWQwOTcgMTAwNjQ0Ci0tLSBhL2RvYy9zcmMv
c2dtbC9wZ3N0YXRzdGF0ZW1lbnRzLnNnbWwKKysrIGIvZG9jL3NyYy9zZ21sL3Bnc3RhdHN0
YXRlbWVudHMuc2dtbApAQCAtOTM2LDYgKzkzNiwyNCBAQAogICAgIDwvbGlzdGl0ZW0+CiAg
ICA8L3Zhcmxpc3RlbnRyeT4KIAorICAgPHZhcmxpc3RlbnRyeT4KKyAgICA8dGVybT4KKyAg
ICAgPHZhcm5hbWU+cGdfc3RhdF9zdGF0ZW1lbnRzLnNhbXBsZV9yYXRlPC92YXJuYW1lPiAo
PHR5cGU+cmVhbDwvdHlwZT4pCisgICAgIDxpbmRleHRlcm0+CisgICAgICA8cHJpbWFyeT48
dmFybmFtZT5wZ19zdGF0X3N0YXRlbWVudHMuc2FtcGxlX3JhdGU8L3Zhcm5hbWU+IGNvbmZp
Z3VyYXRpb24gcGFyYW1ldGVyPC9wcmltYXJ5PgorICAgICA8L2luZGV4dGVybT4KKyAgICA8
L3Rlcm0+CisKKyAgICA8bGlzdGl0ZW0+CisgICAgIDxwYXJhPgorICAgICAgPHZhcm5hbWU+
cGdfc3RhdF9zdGF0ZW1lbnRzLnNhbXBsZV9yYXRlPC92YXJuYW1lPiBjYXVzZXMgcGdfc3Rh
dF9zdGF0ZW1lbnRzIHRvIG9ubHkKKyAgICAgIHRyYWNrIGEgZnJhY3Rpb24gb2YgdGhlIHN0
YXRlbWVudHMgaW4gZWFjaCBzZXNzaW9uLiAgVGhlIGRlZmF1bHQgaXMgMSwKKyAgICAgIG1l
YW5pbmcgdHJhY2sgYWxsIHRoZSBxdWVyaWVzLiBJbiBjYXNlIG9mIG5lc3RlZCBzdGF0ZW1l
bnRzLAorICAgICAgZWl0aGVyIGFsbCB3aWxsIGJlIHRyYWNrZWQgb3Igbm9uZS4gT25seSBz
dXBlcnVzZXJzIGNhbiBjaGFuZ2UgdGhpcyBzZXR0aW5nLgorICAgICA8L3BhcmE+CisgICAg
PC9saXN0aXRlbT4KKyAgIDwvdmFybGlzdGVudHJ5PgorCiAgICA8dmFybGlzdGVudHJ5Pgog
ICAgIDx0ZXJtPgogICAgICA8dmFybmFtZT5wZ19zdGF0X3N0YXRlbWVudHMuc2F2ZTwvdmFy
bmFtZT4gKDx0eXBlPmJvb2xlYW48L3R5cGU+KQotLSAKMi4zNC4xCgo=

--------------XffYrhKbuz6vw56dfTH6ImkU--