Hi, On 2023-09-11 09:23:59 +0300, Karina Litskevich wrote: > On Mon, Sep 11, 2023 at 9:08 AM Karina Litskevich < > litskevichkar...@gmail.com> wrote: > > > I found a bug that causes one of the differences. Wrong counter is > > incremented > > in ExtendBufferedRelLocal(). The attached patch fixes it and should be > > applied > > to REL_16_STABLE and master. > > > > I've forgotten to attach the patch. Here it is.
> From 999a3d533a9b74c8568cc8a3d715c287de45dd2c Mon Sep 17 00:00:00 2001 > From: Karina Litskevich <litskevichkar...@gmail.com> > Date: Thu, 7 Sep 2023 17:44:40 +0300 > Subject: [PATCH v1] Fix local_blks_written counter incrementation > > --- > src/backend/storage/buffer/localbuf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/backend/storage/buffer/localbuf.c > b/src/backend/storage/buffer/localbuf.c > index 1735ec7141..567b8d15ef 100644 > --- a/src/backend/storage/buffer/localbuf.c > +++ b/src/backend/storage/buffer/localbuf.c > @@ -431,7 +431,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr, > > *extended_by = extend_by; > > - pgBufferUsage.temp_blks_written += extend_by; > + pgBufferUsage.local_blks_written += extend_by; > > return first_block; > } > -- > 2.34.1 > Ugh, you're right. The naming of local vs temp here is pretty unfortunate imo. I wonder if we ought to at least dd a comment to BufferUsage clarifying the situation? Just reading the comments therein one would be hard pressed to figure out which of the variables temp table activity should be added to. I don't think we currently can write a test for this in the core tests, as the relevant data isn't visible anywhere, iirc. Thus I added a test to pg_stat_statements. Afaict it should be stable? Running the attached patch through CI, planning to push after that succeeds, unless somebody has a comment? Greetings, Andres Freund
>From ae7b170e9032af0fd257a39953fe0b6ef7e9637c Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Wed, 13 Sep 2023 11:46:45 -0700 Subject: [PATCH v2] Fix tracking of temp table relation extensions as writes Karina figured out that I (Andres) confused BufferUsage.temp_blks_written with BufferUsage.local_blks_written in fcdda1e4b5. Tests in core PG can't easily test this, as BufferUsage is just used for EXPLAIN (ANALYZE, BUFFERS) and pg_stat_statements. Thus this commit adds tests for this to pg_stat_statements. Reported-by: Karina Litskevich <litskevichkar...@gmail.com> Author: Karina Litskevich <litskevichkar...@gmail.com> Author: Andres Freund <and...@anarazel.de> Discussion: https://postgr.es/m/cacit8ibxxa6+0amgikbefhm8b84xdqvo6d0qfd1pq1s8zps...@mail.gmail.com Backpatch: 16-, where fcdda1e4b5 was merged --- src/backend/storage/buffer/localbuf.c | 2 +- contrib/pg_stat_statements/expected/dml.out | 27 +++++++++++++++++++++ contrib/pg_stat_statements/sql/dml.sql | 19 +++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 1735ec71419..567b8d15ef0 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -431,7 +431,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr, *extended_by = extend_by; - pgBufferUsage.temp_blks_written += extend_by; + pgBufferUsage.local_blks_written += extend_by; return first_block; } diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out index 7b9c8f979ee..ede47a71acc 100644 --- a/contrib/pg_stat_statements/expected/dml.out +++ b/contrib/pg_stat_statements/expected/dml.out @@ -139,6 +139,33 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C"; 1 | 1 | SELECT pg_stat_statements_reset() (10 rows) +-- check that [temp] table relation extensions are tracked as writes +CREATE TABLE pgss_extend_tab (a int, b text); +CREATE TEMP TABLE pgss_extend_temp_tab (a int, b text); +SELECT pg_stat_statements_reset(); + pg_stat_statements_reset +-------------------------- + +(1 row) + +INSERT INTO pgss_extend_tab (a, b) SELECT generate_series(1, 1000), 'something'; +INSERT INTO pgss_extend_temp_tab (a, b) SELECT generate_series(1, 1000), 'something'; +WITH sizes AS ( + SELECT + pg_relation_size('pgss_extend_tab') / current_setting('block_size')::int8 AS rel_size, + pg_relation_size('pgss_extend_temp_tab') / current_setting('block_size')::int8 AS temp_rel_size +) +SELECT + SUM(local_blks_written) >= (SELECT temp_rel_size FROM sizes) AS temp_written_ok, + SUM(local_blks_dirtied) >= (SELECT temp_rel_size FROM sizes) AS temp_dirtied_ok, + SUM(shared_blks_written) >= (SELECT rel_size FROM sizes) AS written_ok, + SUM(shared_blks_dirtied) >= (SELECT rel_size FROM sizes) AS dirtied_ok +FROM pg_stat_statements; + temp_written_ok | temp_dirtied_ok | written_ok | dirtied_ok +-----------------+-----------------+------------+------------ + t | t | t | t +(1 row) + SELECT pg_stat_statements_reset(); pg_stat_statements_reset -------------------------- diff --git a/contrib/pg_stat_statements/sql/dml.sql b/contrib/pg_stat_statements/sql/dml.sql index af2f9fcf73b..3b5d2afb858 100644 --- a/contrib/pg_stat_statements/sql/dml.sql +++ b/contrib/pg_stat_statements/sql/dml.sql @@ -73,4 +73,23 @@ MERGE INTO pgss_dml_tab USING pgss_dml_tab st ON (st.a = pgss_dml_tab.a AND st.a DROP TABLE pgss_dml_tab; SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- check that [temp] table relation extensions are tracked as writes +CREATE TABLE pgss_extend_tab (a int, b text); +CREATE TEMP TABLE pgss_extend_temp_tab (a int, b text); +SELECT pg_stat_statements_reset(); +INSERT INTO pgss_extend_tab (a, b) SELECT generate_series(1, 1000), 'something'; +INSERT INTO pgss_extend_temp_tab (a, b) SELECT generate_series(1, 1000), 'something'; +WITH sizes AS ( + SELECT + pg_relation_size('pgss_extend_tab') / current_setting('block_size')::int8 AS rel_size, + pg_relation_size('pgss_extend_temp_tab') / current_setting('block_size')::int8 AS temp_rel_size +) +SELECT + SUM(local_blks_written) >= (SELECT temp_rel_size FROM sizes) AS temp_written_ok, + SUM(local_blks_dirtied) >= (SELECT temp_rel_size FROM sizes) AS temp_dirtied_ok, + SUM(shared_blks_written) >= (SELECT rel_size FROM sizes) AS written_ok, + SUM(shared_blks_dirtied) >= (SELECT rel_size FROM sizes) AS dirtied_ok +FROM pg_stat_statements; + SELECT pg_stat_statements_reset(); -- 2.38.0
