Thread: Improve explicit cursor handling in pg_stat_statements
Hi hackers, I recently looked into a workload that makes heavy use of explicit cursors, and I found that pg_stat_statements can become a bottleneck. The application in question declares hundreds of cursors, and for each one, performs many FETCH and MOVE operations with varying fetch sizes. As a result, pg_stat_statements ends up overwhelmed by the deallocation (and garbage collection) of DECLARE CURSOR, FETCH, and MOVE entries. Each of these is associated with a unique queryId, which leads to bloated entries with limited diagnostic value. Other issues: 1. FETCH/MOVE statements don't really help much in laying blame on a specific query. the DECLARE CURSOR statement could have been evicted in pg_stat_statements by that point or a similar cursor name is pointing to a different query. Also, FETCH doesn't aggregate for the same cursor — e.g., FETCH 10 c1 and FETCH 20 c1 show up as separate entries. 2. DECLARE CURSOR doesn't provide execution stats for the underlying SQL. Enabling pg_stat_statements.track = 'all' can expose the underlying SQL, but adds overhead.There’s also a bug: the toplevel column for the underlying query is still marked as "t", even though you must set track "all" to see it. Based on this, I propose the following improvements: 1. Better normalization of cursor utility commands: 2. Normalize the cursor name in CLOSE. 3. Normalize fetch/move sizes in FETCH and MOVE. Users can use the rows and calls columns to derive average fetch size. Ideally I would want to normalize the cursor name and generate the queryId f the FETCH statement based on the underlying query, but that is not possible to do that post parsing. (The above normalizations of these utility statements will reduce the bloat.) 4. Track the underlying query of a cursor by default, even when pg_stat_statements.track_utility = off. I’ve attached two patches that implement this. Here's a quick example: ``` begin; declare cs1 cursor for select from pg_class; declare cs2 cursor for select from pg_class; fetch 10 cs1; fetch 20 cs1; fetch 10 cs1; fetch 10 cs2; close cs1; close cs2; declare cs1 cursor for select from pg_attribute; SELECT calls, rows, query, toplevel FROM pg_stat_statements ORDER BY query COLLATE "C"; commit; ``` current state: ``` postgres=*# SELECT calls, rows, query, toplevel FROM pg_stat_statements ORDER BY query COLLATE "C"; calls | rows | query | toplevel -------+------+-----------------------------------------------------------------------------------------------------------+---------- 1 | 1 | SELECT name FROM pg_catalog.pg_available_extensions WHERE name LIKE $1 AND installed_version IS NULL+| t | | LIMIT $2 | 1 | 0 | begin | t 1 | 0 | close cs1 | t 1 | 0 | close cs2 | t 1 | 0 | create extension pg_stat_statements | t 1 | 0 | declare cs1 cursor for select from pg_attribute | t 1 | 0 | declare cs1 cursor for select from pg_class | t 1 | 0 | declare cs2 cursor for select from pg_class | t 2 | 20 | fetch 10 cs1 | t 1 | 10 | fetch 10 cs2 | t 1 | 20 | fetch 20 cs1 | t (11 rows) ``` with both patches applied: ``` postgres=*# SELECT calls, rows, query, toplevel FROM pg_stat_statements ORDER BY query COLLATE "C"; calls | rows | query | toplevel -------+------+------------------------------------------------+---------- 1 | 0 | begin | t 2 | 0 | close $1 | t 1 | 0 | declare $1 cursor for select from pg_attribute | t 2 | 0 | declare $1 cursor for select from pg_class | t 3 | 40 | fetch $1 cs1 | t 1 | 10 | fetch $1 cs2 | t 2 | 50 | select from pg_class | t (7 rows) postgres=*# commit; COMMIT ``` FWIW, I raised this ~3 years ago [0], but there was not much interest. I have seen this being a problem a few times since then that I think something should be done about. I also was not happy with the approach I took in [0]. Looking forward to feedback! Regards, -- Sami Imseih Amazon Web Services (AWS) [0] https://www.postgresql.org/message-id/flat/203CFCF7-176E-4AFC-A48E-B2CECFECD6AA%40amazon.com
Attachment
I forgot to add the proper tests to the Normalize cursor utility statements. Reattaching v2. I also want to add that the decision to not normalize the cursor name in the FETCH command is because it would not make sense to combine FETCH commands for various cursors into the same entry. Regards, -- Sami Imseih Amazon Web Services (AWS)
Attachment
On Wed, Apr 30, 2025 at 02:43:41PM -0500, Sami Imseih wrote: > I also want to add that the decision to not normalize the cursor name in > the FETCH command is because it would not make sense to combine > FETCH commands for various cursors into the same entry. - calls | rows | query --------+------+------------------------------------------------------- - 2 | 0 | CLOSE cursor_stats_1 - 2 | 0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1 + calls | rows | query +-------+------+---------------------------------------------------- + 2 | 0 | CLOSE $1 + 2 | 0 | DECLARE $1 CURSOR WITH HOLD FOR SELECT $2 Hmm. What are the workloads that you have seen as problematic? Do these involve cursor names generated randomly, where most of them are similar with a random factor for the name? Too much normalization here would limit the amount of verbosity that we have for this area, especially if we are dealing with query patterns that rely on few CLOSE naming patterns spread across a couple of key queries, because we would now know anymore about their distribution. - 1 | 5 | FETCH FORWARD 5 pgss_cursor - 1 | 7 | FETCH FORWARD ALL pgss_cursor - 1 | 1 | FETCH NEXT pgss_cursor + 1 | 0 | DECLARE $1 CURSOR FOR SELECT * FROM pgss_matv Saying that, applying normalization for the number of FETCH looks like a natural move. It seems to me that we should still make a difference with the ALL case, though? typedef struct ClosePortalStmt { NodeTag type; - char *portalname; /* name of the portal (cursor) */ + /* name of the portal (cursor) */ + char *portalname pg_node_attr(query_jumble_ignore); + ParseLoc location pg_node_attr(query_jumble_location); /* NULL means CLOSE ALL */ Could it matter to make a distinction with CLOSE ALL, compared to the case where the CLOSE statements are named? It would be possible to make a difference compared to the named case with an extra boolean field, for example. I would suggest to add some tests for CLOSE ALL anyway; we don't have any currently. -- Michael
Attachment
> Hmm. What are the workloads that you have seen as problematic? Do > these involve cursor names generated randomly, where most of them are > similar with a random factor for the name? postgres_fdw, as an example, in which cursor name get reused for different queries. Notice below "c1" and "c2" is reused for different queries, so now what underlying sql is FETCH, i.e. FETCH 100 FROM c1 referring to? v2-0001 does not help us with the FETCH problem because as I mentioned we don't have access to the underlying sql ( and parsing is even too early to do a portal lookup to find the underlying sql to base the queryId on). What v2-0001 will do is at least group the DECLARE CURSOR statements together for cursors referencing the same query and reduce the # of entries. ``` create foreign table t2(id int) server r1; create foreign table t1(id int) server r1; postgres=# select * from t2, t ; id | id ----+---- 1 | 1 (1 row) postgres=# select * from t, t2 ; id | id ----+---- 1 | 1 (1 row) ``` on the remote side ``` postgres=# select calls, query from pg_stat_statements where query like '% c%'; calls | query -------+----------------------------------------------------------------- 1 | DECLARE c2 CURSOR FOR + | SELECT id FROM public.t2 2 | DECLARE c1 CURSOR FOR + | SELECT id FROM public.t2 3 | CLOSE c2 3 | CLOSE c1 2 | DECLARE c2 CURSOR FOR + | SELECT id FROM public.t 3 | FETCH 100 FROM c1 3 | FETCH 100 FROM c2 1 | DECLARE c1 CURSOR FOR + | SELECT id FROM public.t 2 | select calls, query from pg_stat_statements where query like $1 (9 rows) ``` > Too much normalization > here would limit the amount of verbosity that we have for this area, > especially if we are dealing with query patterns that rely on few > CLOSE naming patterns spread across a couple of key queries, because > we would now know anymore about their distribution. The FETCH and CLOSE are already not clear to what underlying SQL they are referring to, and there is not much chance to actually improve that unless we track a cursor queryId in pg_stat_statements ( at that point we can show that FETCH or CLOSE refer to this specific cursor statement ). -- Sami
On Fri, May 02, 2025 at 04:21:21PM -0500, Sami Imseih wrote: > postgres_fdw, as an example, in which cursor name get reused > for different queries. Notice below "c1" and "c2" is reused for different > queries, so now what underlying sql is FETCH, i.e. FETCH 100 FROM c1 referring > to? v2-0001 does not help us with the FETCH problem > because as I mentioned we don't have access to the underlying sql > ( and parsing is even too early to do a portal lookup to find the > underlying sql to > base the queryId on). What v2-0001 will do is at least group the DECLARE CURSOR > statements together for cursors referencing the same query and reduce the # > of entries. This case relies on postgres_fdw's GetCursorNumber() that assigns a unique number for a cursor, ensuring uniqueness per connection within a transaction, and the counter is reset at the end of the transactions. So good point for this case that this hurts. If that holds for the most common cases where this is seen as bloating pgss, that brings some solid ground, especially more for applications that use many cursor numbers in long-ish transactions states done under postgres_fdw. I'm still slightly worried about workloads where cursor names could be used to track some balancing of this kind of activity, for example if cursor names are fixed on a transaction-basis depending on the application involved, as that would mean less visibility in the information. Perhaps we would have more confidence by looking closer at drivers or some ORMs that make use of cursors, seeing how the end user would be impacted? That seems hard to measure, though.. > The FETCH and CLOSE are already not clear to what underlying SQL > they are referring to, and there is not much chance to actually > improve that unless > we track a cursor queryId in pg_stat_statements ( at that point we can show that > FETCH or CLOSE refer to this specific cursor statement ). I don't really have an issue for FETCH with the number as the name is still around, but I'm equally worrying about the loss of information for CLOSE that this new normalization would imply. Perhaps my worries don't have a reason to exist here and I'm just a naturally-pessimistic being. -- Michael