On Mon, Mar 6, 2023 at 2:34 PM Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2023-03-06 14:24:09 -0500, Melanie Plageman wrote: > > On Mon, Mar 06, 2023 at 11:09:19AM -0800, Andres Freund wrote: > > > On 2023-03-06 10:09:24 -0500, Melanie Plageman wrote: > > > > On Mon, Mar 6, 2023 at 1:48 AM Kyotaro Horiguchi > > > > <horikyota....@gmail.com> wrote: > > > > > > > > > > At Mon, 06 Mar 2023 15:24:25 +0900 (JST), Kyotaro Horiguchi > > > > > <horikyota....@gmail.com> wrote in > > > > > > In any case, I think we need to avoid such concurrent > > > > > > autovacuum/analyze. > > > > > > > > > > If it is correct, I believe the attached fix works. > > > > > > > > Thanks for investigating this! > > > > > > > > Yes, this fix looks correct and makes sense to me. > > > > > > Wouldn't it be better to just perform the section from the ALTER TABLE > > > till > > > the DROP TABLE in a transaction? Then there couldn't be any other > > > accesses in > > > just that section. I'm not convinced it's good to disallow all concurrent > > > activity in other parts of the test. > > > > You mean for test coverage reasons? Because the table in question only > > exists for a few operations in this test file. > > That, but also because it's simply more reliable. autovacuum=off doesn't > protect against a anti-wraparound vacuum or such. Or a concurrent test somehow > triggering a read. Or ...
Good point. Attached is what you suggested. I committed the transaction before the drop table so that the statistics would be visible when we queried pg_stat_io. - Melanie
From 78ca019624fbd7d6e2a4d94970b804fc834731b4 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <melanieplage...@gmail.com> Date: Mon, 6 Mar 2023 15:16:03 -0500 Subject: [PATCH v1] Fix flakey pg_stat_io test Wrap test of pg_stat_io's tracking of shared buffer reads in a transaction to prevent concurrent accesses (e.g. by autovacuum) leading to incorrect test failures. Discussion: https://www.postgresql.org/message-id/20230306190919.ai6mxdq3sygyyths%40awork3.anarazel.de --- src/test/regress/expected/stats.out | 4 ++++ src/test/regress/sql/stats.sql | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 937b2101b3..fb5adb0fd7 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1181,6 +1181,9 @@ SELECT current_setting('fsync') = 'off' -- from it to cause it to be read back into shared buffers. SELECT sum(reads) AS io_sum_shared_before_reads FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset +-- Do this in a transaction to prevent any other concurrent access to our newly +-- rewritten table, guaranteeing our test will pass. +BEGIN; ALTER TABLE test_io_shared SET TABLESPACE regress_tblspace; -- SELECT from the table so that the data is read into shared buffers and -- io_context 'normal', io_object 'relation' reads are counted. @@ -1190,6 +1193,7 @@ SELECT COUNT(*) FROM test_io_shared; 100 (1 row) +COMMIT; SELECT pg_stat_force_next_flush(); pg_stat_force_next_flush -------------------------- diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index 74e592aa8a..84604d8fa0 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -576,10 +576,14 @@ SELECT current_setting('fsync') = 'off' -- from it to cause it to be read back into shared buffers. SELECT sum(reads) AS io_sum_shared_before_reads FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset +-- Do this in a transaction to prevent any other concurrent access to our newly +-- rewritten table, guaranteeing our test will pass. +BEGIN; ALTER TABLE test_io_shared SET TABLESPACE regress_tblspace; -- SELECT from the table so that the data is read into shared buffers and -- io_context 'normal', io_object 'relation' reads are counted. SELECT COUNT(*) FROM test_io_shared; +COMMIT; SELECT pg_stat_force_next_flush(); SELECT sum(reads) AS io_sum_shared_after_reads FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset -- 2.37.2
