Subject: Cancel problems of query to pg_stat_statements

From: Roman Khapov <[email-masked]>
Date: Thu, 24 Apr 2025 22:49:16 +0500

Hi!

Recently we faced a problem in out production psql installation, which was that we had to cancel all requests to the db, including performance monitoring requests, that uses ps_stat_statements. But we could not cancel the request in usual way, and had to kill -9 the pg process of it.

We've noticed that the the query execution stuck on PGSS_TEXT_FILE file reading in function qtext_load_file, which doesn't have CHECK_FOR_INTERRUPTS in the read cycle. In addition to our case with large PGSS_TEXT_FILE (and maybe the problems with virtual disk i/o) that can explain uncancellable pg_stat_statements queries.

Also, the reading block size can be reduced from 1GB to 32MB in order to increase the frequency of CHECK_FOR_INTERRUPTS calls without qtext_load_file performance degradation. To check that I did some little testing with fio like:

fio --name=readtest --filename=./random-bytes-file --rw=read --bs=32m --size=10G --ioengine=libaio --direct=1 --numjobs=1

So, I made a simple patch that adds CHECK_FOR_INTERRUPTS call in read cycle of qtext_load_file and change max value of toread from 1GB to 32MB.

I would appreciate your feedback on these changes.

From eab041fcd1f4973b03f6bebd37bef3395fe64b4b Mon Sep 17 00:00:00 2001
From: rkhapov <[email-masked]>
Date: Thu, 24 Apr 2025 17:01:54 +0000
Subject: [PATCH] pg_stat_statements.c: cancelable qtext_load_file

In the case of a large PGSS_TEXT_FILE, the work time of the qtext_load_file
function will be quite long, and the query to the pg_stat_statements table
will not be cancellable, as there is no CHECK_FOR_INTERRUPT in the function.

Also, the amount of bytes read can reach 1 GB, which leads to a slow read
system call that does not allow cancellation of the query. Testing the speed
of sequential read using fio with different block sizes shows that there is
no significant difference between 16 MB blocks and 1 GB blocks.

Therefore, this patch changes the maximum read value from 1 GB to 16 MB and
adds CHECK_FOR_INTERRUPTION in the read loop of qtext_load_file to make it cancellable.

Signed-off-by: rkhapov <[email-masked]>
---
 contrib/pg_stat_statements/pg_stat_statements.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 9778407cba3..cd34f1ce248 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2365,7 +2365,9 @@ qtext_load_file(Size *buffer_size)
 	nread = 0;
 	while (nread < stat.st_size)
 	{
-		int			toread = Min(1024 * 1024 * 1024, stat.st_size - nread);
+		int			toread = Min(32 * 1024 * 1024, stat.st_size - nread);
+
+		CHECK_FOR_INTERRUPTS();
 
 		/*
 		 * If we get a short read and errno doesn't get set, the reason is
-- 
2.43.0

---

From: Andrey Borodin <[email-masked]>
Date: Mon, 28 Apr 2025 11:16:19 +0500

> On 24 Apr 2025, at 22:49, Roman Khapov <[email-masked]> wrote:
> 
> Hi!

Interesting problem, thanks for raising it!

>  We've noticed that the the query execution stuck on PGSS_TEXT_FILE file reading in function qtext_load_file, which doesn't have CHECK_FOR_INTERRUPTS in the read cycle. In addition to our case with…

I'm afraid it might be not so easy to add CHECK_FOR_INTERRUPTS there. Most probably you was holding a LWLockAcquire(pgss->lock, LW_SHARED) somewhere (do you have a backtrace?), which prevent interrupts anyway.

Thanks!

Best regards, Andrey Borodin.

---

From: Roman Khapov <[email-masked]>
Date: Mon, 12 May 2025 15:48:29 +0500

Hi! Thanks for the review!

Its true that qtext_load_file() can be called with acquired lock in pg_stat_statements_internal()…
So I perform INTERRUPTS_PENDING_CONDITION in v2 patch and call CHECK_FOR_INTERRUPTS later, after cycle ended and lock released.

> On 28 Apr 2025, at 11:16, Andrey Borodin <[email-masked]> wrote:
> 
> 
>

---

From: Michael Paquier <[email-masked]>
Date: Wed, 25 Mar 2026 14:11:00 +0900

On Mon, May 12, 2025 at 03:48:29PM +0500, Roman Khapov wrote:
> Its true that qtext_load_file() can be called with acquired lock in
> pg_stat_statements_internal()...
> So I perform INTERRUPTS_PENDING_CONDITION in v2 patch and call
> CHECK_FOR_INTERRUPTS later, after cycle ended and lock released.

This patch is only a workaround for a larger problem: the PGSS text
file does not and will never scale once it reaches a large size and
once we have a high turnover rate of the PGSS entries due to different
query IDs.

FWIW, Sami Imseih has mentioned to me a few days ago that the state of
the pgstats API (that can be used for custom stats kinds as well) was
basically in a shape good enough to move PGSS to pgstats as a custom
kind, and we should be able to move the query text file to be entirely
in-memory.  This proposal will hopefully materialize at the beginning
of the v20 cycle.
--
Michael