pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedAdd pg_freespacemap extension sql test
17+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2022-03-29 c14a9eeec4 Avoid instabilities with the regression tests of pg_freespacemap
2022-03-24 a1bc4d3590 Add some basic regression tests for pg_freespacemap
* Add pg_freespacemap extension sql test
@ 2022-03-08 14:39 Dong Wook Lee <sh95119@gmail.com>
0 siblings, 2 replies; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-08 14:39 UTC (permalink / raw)
To: pgsql-hackers
Hi,
I just added some tests for the pg_freespacemap extension because the test
coverage was 0 percent.
But I don't know if I did it correctly.
---
Regards
Lee Dong Wook
Attachments:
0001_add_link_fsm.patch (application/octet-stream, 653B) download
diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml
index 5025498249..0ab3307e9c 100644
--- a/doc/src/sgml/pgfreespacemap.sgml
+++ b/doc/src/sgml/pgfreespacemap.sgml
@@ -9,7 +9,7 @@
<para>
The <filename>pg_freespacemap</filename> module provides a means for examining the
- free space map (FSM). It provides a function called
+ <link linkend="storage-fsm">free space map</link> (FSM). It provides a function called
<function>pg_freespace</function>, or two overloaded functions, to be
precise. The functions show the value recorded in the free space map for
a given page, or for all pages in the relation.
* Re: Add pg_freespacemap extension sql test
@ 2022-03-08 14:43 Dong Wook Lee <sh95119@gmail.com>
parent: Dong Wook Lee <sh95119@gmail.com>
1 sibling, 1 reply; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-08 14:43 UTC (permalink / raw)
To: pgsql-hackers
I'm sorry for attaching the wrong patch file.
2022년 3월 8일 (화) 오후 11:39, Dong Wook Lee <sh95119@gmail.com>님이 작성:
> Hi,
> I just added some tests for the pg_freespacemap extension because the test
> coverage was 0 percent.
> But I don't know if I did it correctly.
>
> ---
> Regards
> Lee Dong Wook
>
Attachments:
0001_add_test_pg_fsm.patch (application/octet-stream, 2.0K) download
diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile
index da40b80c7c..2d525a1284 100644
--- a/contrib/pg_freespacemap/Makefile
+++ b/contrib/pg_freespacemap/Makefile
@@ -10,6 +10,8 @@ DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \
pg_freespacemap--1.0--1.1.sql
PGFILEDESC = "pg_freespacemap - monitoring of free space map"
+REGRESS = pg_freespacemap
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_freespacemap/expected/pg_freespacemap.out b/contrib/pg_freespacemap/expected/pg_freespacemap.out
new file mode 100644
index 0000000000..fc975b9050
--- /dev/null
+++ b/contrib/pg_freespacemap/expected/pg_freespacemap.out
@@ -0,0 +1,36 @@
+CREATE EXTENSION pg_freespacemap;
+CREATE TABLE t1(c1 int);
+INSERT INTO t1 VALUES (generate_series(1, 1000));
+VACUUM t1;
+SELECT * FROM pg_freespace('t1');
+ blkno | avail
+-------+-------
+ 0 | 0
+ 1 | 0
+ 2 | 0
+ 3 | 0
+ 4 | 4704
+(5 rows)
+
+DELETE FROM t1 WHERE c1 <= 10;
+VACUUM t1;
+SELECT * FROM pg_freespace('t1');
+ blkno | avail
+-------+-------
+ 0 | 320
+ 1 | 0
+ 2 | 0
+ 3 | 0
+ 4 | 4704
+(5 rows)
+
+SELECT * FROM pg_freespace('t1', 0);
+ pg_freespace
+--------------
+ 320
+(1 row)
+
+SELECT * FROM pg_freespace('t1', -1);
+ERROR: invalid block number
+SELECT * FROM pg_freespace('t1', 4294967295);
+ERROR: invalid block number
diff --git a/contrib/pg_freespacemap/sql/pg_freespacemap.sql b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
new file mode 100644
index 0000000000..cc175322cd
--- /dev/null
+++ b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
@@ -0,0 +1,17 @@
+CREATE EXTENSION pg_freespacemap;
+
+CREATE TABLE t1(c1 int);
+
+INSERT INTO t1 VALUES (generate_series(1, 1000));
+VACUUM t1;
+
+SELECT * FROM pg_freespace('t1');
+
+DELETE FROM t1 WHERE c1 <= 10;
+VACUUM t1;
+
+SELECT * FROM pg_freespace('t1');
+SELECT * FROM pg_freespace('t1', 0);
+
+SELECT * FROM pg_freespace('t1', -1);
+SELECT * FROM pg_freespace('t1', 4294967295);
* Re: Add pg_freespacemap extension sql test
@ 2022-03-08 14:45 Justin Pryzby <pryzby@telsasoft.com>
parent: Dong Wook Lee <sh95119@gmail.com>
1 sibling, 1 reply; 17+ messages in thread
From: Justin Pryzby @ 2022-03-08 14:45 UTC (permalink / raw)
To: Dong Wook Lee <sh95119@gmail.com>; +Cc: pgsql-hackers
On Tue, Mar 08, 2022 at 11:39:08PM +0900, Dong Wook Lee wrote:
> Hi,
> I just added some tests for the pg_freespacemap extension because the test
> coverage was 0 percent.
> But I don't know if I did it correctly.
The patch only touches doc/*.sgml.
I suppose you forgot to use "git add".
--
Justin
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-08 14:54 Dong Wook Lee <sh95119@gmail.com>
parent: Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 0 replies; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-08 14:54 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: pgsql-hackers
That's right, so I attached the correct file again.
2022년 3월 8일 (화) 오후 11:45, Justin Pryzby <pryzby@telsasoft.com>님이 작성:
> On Tue, Mar 08, 2022 at 11:39:08PM +0900, Dong Wook Lee wrote:
> > Hi,
> > I just added some tests for the pg_freespacemap extension because the
> test
> > coverage was 0 percent.
> > But I don't know if I did it correctly.
>
> The patch only touches doc/*.sgml.
> I suppose you forgot to use "git add".
>
> --
> Justin
>
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-08 16:19 Tom Lane <tgl@sss.pgh.pa.us>
parent: Dong Wook Lee <sh95119@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2022-03-08 16:19 UTC (permalink / raw)
To: Dong Wook Lee <sh95119@gmail.com>; +Cc: pgsql-hackers
Dong Wook Lee <sh95119@gmail.com> writes:
> [ 0001_add_test_pg_fsm.patch ]
I think having some coverage here would be great, but I'm concerned that
this patch doesn't look very portable. Aren't the numbers liable to
change on 32-bit machines, in particular?
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-09 11:13 Dong Wook Lee <sh95119@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-09 11:13 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
2022년 3월 9일 (수) 오전 1:19, Tom Lane <tgl@sss.pgh.pa.us>님이 작성:
> Dong Wook Lee <sh95119@gmail.com> writes:
> > [ 0001_add_test_pg_fsm.patch ]
>
> I think having some coverage here would be great, but I'm concerned that
> this patch doesn't look very portable. Aren't the numbers liable to
> change on 32-bit machines, in particular?
>
> regards, tom lane
>
I agree with you, but I have no good idea how to deal with it.
Can the Perl TAP test be a good way?
Thought?
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-11 05:51 Michael Paquier <michael@paquier.xyz>
parent: Dong Wook Lee <sh95119@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2022-03-11 05:51 UTC (permalink / raw)
To: Dong Wook Lee <sh95119@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers
On Wed, Mar 09, 2022 at 08:13:15PM +0900, Dong Wook Lee wrote:
> I agree with you, but I have no good idea how to deal with it.
Well, my guess is that you basically just care about being able to
detect if there is free space in the map or not, which goes down to
detecting if pg_freespace() returns 0 or a number strictly higher than
0, so wouldn't it be enough to stick some > 0 in your test queries?
Btw, if you want to test 32-bit builds, gcc allows that by passing
down -m32.
> Can the Perl TAP test be a good way?
That does not seem necessary here.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-15 13:50 Dong Wook Lee <sh95119@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-15 13:50 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
2022년 3월 11일 (금) 오후 2:51, Michael Paquier <michael@paquier.xyz>님이 작성:
>
> On Wed, Mar 09, 2022 at 08:13:15PM +0900, Dong Wook Lee wrote:
> > I agree with you, but I have no good idea how to deal with it.
>
> Well, my guess is that you basically just care about being able to
> detect if there is free space in the map or not, which goes down to
> detecting if pg_freespace() returns 0 or a number strictly higher than
> 0, so wouldn't it be enough to stick some > 0 in your test queries?
> Btw, if you want to test 32-bit builds, gcc allows that by passing
> down -m32.
>
> > Can the Perl TAP test be a good way?
>
> That does not seem necessary here.
> --
> Michael
so, you mean it's not necessary to add cases for negative numbers or
beyond the range?
I just wrote down testable cases, and if it doesn't have a big
advantage, I don't mind not adding that case.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-19 16:18 Dong Wook Lee <sh95119@gmail.com>
parent: Dong Wook Lee <sh95119@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-19 16:18 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>
> Well, my guess is that you basically just care about being able to
> detect if there is free space in the map or not, which goes down to
> detecting if pg_freespace() returns 0 or a number strictly higher than
> 0, so wouldn't it be enough to stick some > 0 in your test queries?
I edited the previous patch file.
Am I correct in understanding that?
Attachments:
0002_add_test_pg_fsm.patch (application/octet-stream, 1.5K) download
diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile
index da40b80c7c..23cfb67c35 100644
--- a/contrib/pg_freespacemap/Makefile
+++ b/contrib/pg_freespacemap/Makefile
@@ -9,6 +9,7 @@ EXTENSION = pg_freespacemap
DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \
pg_freespacemap--1.0--1.1.sql
PGFILEDESC = "pg_freespacemap - monitoring of free space map"
+REGRESS = pg_freespacemap
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/contrib/pg_freespacemap/expected/pg_freespacemap.out b/contrib/pg_freespacemap/expected/pg_freespacemap.out
new file mode 100644
index 0000000000..838936bb39
--- /dev/null
+++ b/contrib/pg_freespacemap/expected/pg_freespacemap.out
@@ -0,0 +1,16 @@
+CREATE EXTENSION pg_freespacemap;
+CREATE TABLE t1(c1 int);
+INSERT INTO t1 VALUES (generate_series(1, 100));
+VACUUM t1;
+SELECT * FROM pg_freespace('t1');
+ blkno | avail
+-------+-------
+ 0 | 4544
+(1 row)
+
+SELECT * FROM pg_freespace('t1', 0);
+ pg_freespace
+--------------
+ 4544
+(1 row)
+
diff --git a/contrib/pg_freespacemap/sql/pg_freespacemap.sql b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
new file mode 100644
index 0000000000..f69f767811
--- /dev/null
+++ b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
@@ -0,0 +1,9 @@
+CREATE EXTENSION pg_freespacemap;
+
+CREATE TABLE t1(c1 int);
+
+INSERT INTO t1 VALUES (generate_series(1, 100));
+VACUUM t1;
+
+SELECT * FROM pg_freespace('t1');
+SELECT * FROM pg_freespace('t1', 0);
* Re: Add pg_freespacemap extension sql test
@ 2022-03-19 18:13 Fabrízio de Royes Mello <fabriziomello@gmail.com>
parent: Dong Wook Lee <sh95119@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Fabrízio de Royes Mello @ 2022-03-19 18:13 UTC (permalink / raw)
To: Dong Wook Lee <sh95119@gmail.com>; +Cc: pgsql-hackers; Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>
On Sat, Mar 19, 2022 at 1:18 PM Dong Wook Lee <sh95119@gmail.com> wrote:
>
> > Well, my guess is that you basically just care about being able to
> > detect if there is free space in the map or not, which goes down to
> > detecting if pg_freespace() returns 0 or a number strictly higher than
> > 0, so wouldn't it be enough to stick some > 0 in your test queries?
>
> I edited the previous patch file.
> Am I correct in understanding that?
>
I think what Michael meant is something like attached.
Regards,
--
Fabrízio de Royes Mello
Attachments:
0003_add_test_pg_fsm.patch (text/x-patch, 2.1K) download
diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile
index da40b80c7c..2d525a1284 100644
--- a/contrib/pg_freespacemap/Makefile
+++ b/contrib/pg_freespacemap/Makefile
@@ -10,6 +10,8 @@ DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \
pg_freespacemap--1.0--1.1.sql
PGFILEDESC = "pg_freespacemap - monitoring of free space map"
+REGRESS = pg_freespacemap
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_freespacemap/expected/pg_freespacemap.out b/contrib/pg_freespacemap/expected/pg_freespacemap.out
new file mode 100644
index 0000000000..9bf8055247
--- /dev/null
+++ b/contrib/pg_freespacemap/expected/pg_freespacemap.out
@@ -0,0 +1,36 @@
+CREATE EXTENSION pg_freespacemap;
+CREATE TABLE t1(c1 int);
+INSERT INTO t1 VALUES (generate_series(1, 1000));
+VACUUM t1;
+SELECT blkno, avail > 0 AS exists FROM pg_freespace('t1');
+ blkno | exists
+-------+--------
+ 0 | f
+ 1 | f
+ 2 | f
+ 3 | f
+ 4 | t
+(5 rows)
+
+DELETE FROM t1 WHERE c1 <= 10;
+VACUUM t1;
+SELECT blkno, avail > 0 AS exists FROM pg_freespace('t1');
+ blkno | exists
+-------+--------
+ 0 | t
+ 1 | f
+ 2 | f
+ 3 | f
+ 4 | t
+(5 rows)
+
+SELECT freespace > 0 FROM pg_freespace('t1', 0) AS freespace;
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT * FROM pg_freespace('t1', -1);
+ERROR: invalid block number
+SELECT * FROM pg_freespace('t1', 4294967295);
+ERROR: invalid block number
diff --git a/contrib/pg_freespacemap/sql/pg_freespacemap.sql b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
new file mode 100644
index 0000000000..f416a29f12
--- /dev/null
+++ b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
@@ -0,0 +1,17 @@
+CREATE EXTENSION pg_freespacemap;
+
+CREATE TABLE t1(c1 int);
+
+INSERT INTO t1 VALUES (generate_series(1, 1000));
+VACUUM t1;
+
+SELECT blkno, avail > 0 AS exists FROM pg_freespace('t1');
+
+DELETE FROM t1 WHERE c1 <= 10;
+VACUUM t1;
+
+SELECT blkno, avail > 0 AS exists FROM pg_freespace('t1');
+SELECT freespace > 0 FROM pg_freespace('t1', 0) AS freespace;
+
+SELECT * FROM pg_freespace('t1', -1);
+SELECT * FROM pg_freespace('t1', 4294967295);
* Re: Add pg_freespacemap extension sql test
@ 2022-03-21 12:12 Dong Wook Lee <sh95119@gmail.com>
parent: Fabrízio de Royes Mello <fabriziomello@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Dong Wook Lee @ 2022-03-21 12:12 UTC (permalink / raw)
To: fabriziomello@gmail.com; +Cc: pgsql-hackers
2022년 3월 20일 (일) 03:13, Fabrízio de Royes Mello <fabriziomello@gmail.com>님이
작성:
>
>
> On Sat, Mar 19, 2022 at 1:18 PM Dong Wook Lee <sh95119@gmail.com> wrote:
> >
> > > Well, my guess is that you basically just care about being able to
> > > detect if there is free space in the map or not, which goes down to
> > > detecting if pg_freespace() returns 0 or a number strictly higher than
> > > 0, so wouldn't it be enough to stick some > 0 in your test queries?
> >
> > I edited the previous patch file.
> > Am I correct in understanding that?
> >
>
> I think what Michael meant is something like attached.
>
> Regards,
>
> --
> Fabrízio de Royes Mello
>
I think you’re right, thank you for sending it instead of me.
>
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-23 06:05 Michael Paquier <michael@paquier.xyz>
parent: Dong Wook Lee <sh95119@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2022-03-23 06:05 UTC (permalink / raw)
To: Dong Wook Lee <sh95119@gmail.com>; +Cc: fabriziomello@gmail.com; pgsql-hackers
On Mon, Mar 21, 2022 at 09:12:37PM +0900, Dong Wook Lee wrote:
> 2022년 3월 20일 (일) 03:13, Fabrízio de Royes Mello <fabriziomello@gmail.com>님이
> 작성:
>> On Sat, Mar 19, 2022 at 1:18 PM Dong Wook Lee <sh95119@gmail.com> wrote:
>>>> Well, my guess is that you basically just care about being able to
>>>> detect if there is free space in the map or not, which goes down to
>>>> detecting if pg_freespace() returns 0 or a number strictly higher than
>>>> 0, so wouldn't it be enough to stick some > 0 in your test queries?
>>>
>>> I edited the previous patch file.
>>> Am I correct in understanding that?
>>>
>>
>> I think what Michael meant is something like attached.
>
> I think you’re right, thank you for sending it instead of me.
Yes, something like v3 was what I was referring to as we cannot rely
on exact numbers for this test suite. At least, we can check if there
is a FSM for a given block, even if that can be limited.
After review, I don't like much the idea of allowing concurrent
autovacuums to run in parallel of the table(s) of this test, so we'd
better disable it explicitely. "t1" is also a very generic name to
use in a regression test. Another thing that itched me is that we
could also test more with indexes, particularly with btree, BRIN and
hash (the latter should not have a FSM with 10 pages as per the first
group batch, and each one has a stable an initial state). Finally,
making the tests stable across 32-bit compilations (say gcc -m32) is
proving to be tricky, but it should be safe enough to check if the FSM
is computed or not with a minimal number of tuples.
Btw, a .gitignore was also forgotten.
I have extended the set of tests as of the attached, running these
across everything I could (CI, all my hosts including Windows, macos,
Linux). We could do more later, of course, but this looks enough to
me as a first step. And I think that this will not upset the
buildfarm.
--
Michael
Attachments:
v4-0001-Add-some-regression-tests-for-pg_freespacemap.patch (text/x-diff, 6.1K) download
From 10002eaed60e3128cfe93c3908254eed81341e52 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 23 Mar 2022 15:00:13 +0900
Subject: [PATCH v4] Add some regression tests for pg_freespacemap
---
contrib/pg_freespacemap/.gitignore | 4 +
contrib/pg_freespacemap/Makefile | 2 +
.../expected/pg_freespacemap.out | 85 +++++++++++++++++++
.../pg_freespacemap/sql/pg_freespacemap.sql | 32 +++++++
4 files changed, 123 insertions(+)
create mode 100644 contrib/pg_freespacemap/.gitignore
create mode 100644 contrib/pg_freespacemap/expected/pg_freespacemap.out
create mode 100644 contrib/pg_freespacemap/sql/pg_freespacemap.sql
diff --git a/contrib/pg_freespacemap/.gitignore b/contrib/pg_freespacemap/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/contrib/pg_freespacemap/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile
index da40b80c7c..2d525a1284 100644
--- a/contrib/pg_freespacemap/Makefile
+++ b/contrib/pg_freespacemap/Makefile
@@ -10,6 +10,8 @@ DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \
pg_freespacemap--1.0--1.1.sql
PGFILEDESC = "pg_freespacemap - monitoring of free space map"
+REGRESS = pg_freespacemap
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_freespacemap/expected/pg_freespacemap.out b/contrib/pg_freespacemap/expected/pg_freespacemap.out
new file mode 100644
index 0000000000..c0f71ba8aa
--- /dev/null
+++ b/contrib/pg_freespacemap/expected/pg_freespacemap.out
@@ -0,0 +1,85 @@
+CREATE EXTENSION pg_freespacemap;
+CREATE TABLE freespace_tab (c1 int) WITH (autovacuum_enabled = off);
+CREATE INDEX freespace_brin ON freespace_tab USING brin (c1);
+CREATE INDEX freespace_btree ON freespace_tab USING btree (c1);
+CREATE INDEX freespace_hash ON freespace_tab USING hash (c1);
+-- Report all the sizes of FSMs for all the relation blocks involved here.
+WITH rel AS (SELECT oid::regclass AS id FROM pg_class WHERE relname ~ 'freespace')
+ SELECT rel.id, fsm.blkno, (fsm.avail > 0) AS is_avail
+ FROM rel, LATERAL pg_freespace(rel.id) AS fsm
+ ORDER BY 1, 2;
+ id | blkno | is_avail
+-----------------+-------+----------
+ freespace_brin | 0 | f
+ freespace_brin | 1 | f
+ freespace_brin | 2 | t
+ freespace_btree | 0 | f
+ freespace_hash | 0 | f
+ freespace_hash | 1 | f
+ freespace_hash | 2 | f
+ freespace_hash | 3 | f
+ freespace_hash | 4 | f
+ freespace_hash | 5 | f
+ freespace_hash | 6 | f
+ freespace_hash | 7 | f
+ freespace_hash | 8 | f
+ freespace_hash | 9 | f
+(14 rows)
+
+INSERT INTO freespace_tab VALUES (1);
+VACUUM freespace_tab;
+WITH rel AS (SELECT oid::regclass AS id FROM pg_class WHERE relname ~ 'freespace')
+ SELECT rel.id, fsm.blkno, (fsm.avail > 0) AS is_avail
+ FROM rel, LATERAL pg_freespace(rel.id) AS fsm
+ ORDER BY 1, 2;
+ id | blkno | is_avail
+-----------------+-------+----------
+ freespace_tab | 0 | t
+ freespace_brin | 0 | f
+ freespace_brin | 1 | f
+ freespace_brin | 2 | t
+ freespace_btree | 0 | f
+ freespace_btree | 1 | f
+ freespace_hash | 0 | f
+ freespace_hash | 1 | f
+ freespace_hash | 2 | f
+ freespace_hash | 3 | f
+ freespace_hash | 4 | f
+ freespace_hash | 5 | f
+ freespace_hash | 6 | f
+ freespace_hash | 7 | f
+ freespace_hash | 8 | f
+ freespace_hash | 9 | f
+(16 rows)
+
+DELETE FROM freespace_tab;
+VACUUM freespace_tab;
+WITH rel AS (SELECT oid::regclass AS id FROM pg_class WHERE relname ~ 'freespace')
+ SELECT rel.id, fsm.blkno, (fsm.avail > 0) AS is_avail
+ FROM rel, LATERAL pg_freespace(rel.id) AS fsm
+ ORDER BY 1, 2;
+ id | blkno | is_avail
+-----------------+-------+----------
+ freespace_brin | 0 | f
+ freespace_brin | 1 | f
+ freespace_brin | 2 | t
+ freespace_btree | 0 | f
+ freespace_btree | 1 | f
+ freespace_hash | 0 | f
+ freespace_hash | 1 | f
+ freespace_hash | 2 | f
+ freespace_hash | 3 | f
+ freespace_hash | 4 | f
+ freespace_hash | 5 | f
+ freespace_hash | 6 | f
+ freespace_hash | 7 | f
+ freespace_hash | 8 | f
+ freespace_hash | 9 | f
+(15 rows)
+
+-- failures with incorrect block number
+SELECT * FROM pg_freespace('freespace_tab', -1);
+ERROR: invalid block number
+SELECT * FROM pg_freespace('freespace_tab', 4294967295);
+ERROR: invalid block number
+DROP TABLE freespace_tab;
diff --git a/contrib/pg_freespacemap/sql/pg_freespacemap.sql b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
new file mode 100644
index 0000000000..b8d0405c63
--- /dev/null
+++ b/contrib/pg_freespacemap/sql/pg_freespacemap.sql
@@ -0,0 +1,32 @@
+CREATE EXTENSION pg_freespacemap;
+
+CREATE TABLE freespace_tab (c1 int) WITH (autovacuum_enabled = off);
+CREATE INDEX freespace_brin ON freespace_tab USING brin (c1);
+CREATE INDEX freespace_btree ON freespace_tab USING btree (c1);
+CREATE INDEX freespace_hash ON freespace_tab USING hash (c1);
+
+-- Report all the sizes of FSMs for all the relation blocks involved here.
+WITH rel AS (SELECT oid::regclass AS id FROM pg_class WHERE relname ~ 'freespace')
+ SELECT rel.id, fsm.blkno, (fsm.avail > 0) AS is_avail
+ FROM rel, LATERAL pg_freespace(rel.id) AS fsm
+ ORDER BY 1, 2;
+
+INSERT INTO freespace_tab VALUES (1);
+VACUUM freespace_tab;
+WITH rel AS (SELECT oid::regclass AS id FROM pg_class WHERE relname ~ 'freespace')
+ SELECT rel.id, fsm.blkno, (fsm.avail > 0) AS is_avail
+ FROM rel, LATERAL pg_freespace(rel.id) AS fsm
+ ORDER BY 1, 2;
+
+DELETE FROM freespace_tab;
+VACUUM freespace_tab;
+WITH rel AS (SELECT oid::regclass AS id FROM pg_class WHERE relname ~ 'freespace')
+ SELECT rel.id, fsm.blkno, (fsm.avail > 0) AS is_avail
+ FROM rel, LATERAL pg_freespace(rel.id) AS fsm
+ ORDER BY 1, 2;
+
+-- failures with incorrect block number
+SELECT * FROM pg_freespace('freespace_tab', -1);
+SELECT * FROM pg_freespace('freespace_tab', 4294967295);
+
+DROP TABLE freespace_tab;
--
2.35.1
* Re: Add pg_freespacemap extension sql test
@ 2022-03-23 13:45 Fabrízio de Royes Mello <fabriziomello@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 17+ messages in thread
From: Fabrízio de Royes Mello @ 2022-03-23 13:45 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Dong Wook Lee <sh95119@gmail.com>; pgsql-hackers
On Wed, Mar 23, 2022 at 3:05 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> After review, I don't like much the idea of allowing concurrent
> autovacuums to run in parallel of the table(s) of this test, so we'd
> better disable it explicitely.
Make sense.
> "t1" is also a very generic name to use in a regression test.
Agreed!
> Another thing that itched me is that we
> could also test more with indexes, particularly with btree, BRIN and
> hash (the latter should not have a FSM with 10 pages as per the first
> group batch, and each one has a stable an initial state).
What about GIN/GIST indexes?
> I have extended the set of tests as of the attached, running these
> across everything I could (CI, all my hosts including Windows, macos,
> Linux). We could do more later, of course, but this looks enough to
> me as a first step. And I think that this will not upset the
> buildfarm.
Also LGTM.
Regards,
--
Fabrízio de Royes Mello
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-24 00:40 Michael Paquier <michael@paquier.xyz>
parent: Fabrízio de Royes Mello <fabriziomello@gmail.com>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2022-03-24 00:40 UTC (permalink / raw)
To: Fabrízio de Royes Mello <fabriziomello@gmail.com>; +Cc: Dong Wook Lee <sh95119@gmail.com>; pgsql-hackers
On Wed, Mar 23, 2022 at 10:45:19AM -0300, Fabrízio de Royes Mello wrote:
> On Wed, Mar 23, 2022 at 3:05 AM Michael Paquier <michael@paquier.xyz> wrote:
>> Another thing that itched me is that we
>> could also test more with indexes, particularly with btree, BRIN and
>> hash (the latter should not have a FSM with 10 pages as per the first
>> group batch, and each one has a stable an initial state).
>
> What about GIN/GIST indexes?
Yes, we could extend that more. For now, I am curious to see what the
buildfarm has to say with the current contents of the patch, and I can
keep an eye on the buildfarm today, so I have applied it.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-27 17:18 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2022-03-27 17:18 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Fabrízio de Royes Mello <fabriziomello@gmail.com>; Dong Wook Lee <sh95119@gmail.com>; pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes:
> Yes, we could extend that more. For now, I am curious to see what the
> buildfarm has to say with the current contents of the patch, and I can
> keep an eye on the buildfarm today, so I have applied it.
It seems this is unstable under valgrind [1]:
inline patch 1: /mnt/resource/bf/build/skink-master/HEAD/pgsql.build/contrib/pg_freespacemap/results/pg_freespacemap.out (+2/-1 lines) download inline-1.patch
--- /mnt/resource/bf/build/skink-master/HEAD/pgsql/contrib/pg_freespacemap/expected/pg_freespacemap.out 2022-03-24 09:39:43.974477703 +0000
+++ /mnt/resource/bf/build/skink-master/HEAD/pgsql.build/contrib/pg_freespacemap/results/pg_freespacemap.out 2022-03-27 17:07:23.896287669 +0000
@@ -60,6 +60,7 @@
ORDER BY 1, 2;
id | blkno | is_avail
-----------------+-------+----------
+ freespace_tab | 0 | t
freespace_brin | 0 | f
freespace_brin | 1 | f
freespace_brin | 2 | t
@@ -75,7 +76,7 @@
freespace_hash | 7 | f
freespace_hash | 8 | f
freespace_hash | 9 | f
-(15 rows)
+(16 rows)
-- failures with incorrect block number
SELECT * FROM pg_freespace('freespace_tab', -1);
* Re: Add pg_freespacemap extension sql test
@ 2022-03-28 03:12 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2022-03-28 03:12 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Fabrízio de Royes Mello <fabriziomello@gmail.com>; Dong Wook Lee <sh95119@gmail.com>; pgsql-hackers
On Sun, Mar 27, 2022 at 01:18:46PM -0400, Tom Lane wrote:
> skink has passed several runs since the commit went in, so it's
> "unstable" not "fails consistently". I see the test tries to
> disable autovacuum on that table, so that doesn't seem to be
> the problem ... what is?
This is a race condition, directly unrelated to valgrind but easier to
trigger under it because things get slower. It takes me a dozen of
tries to be able to reproduce the failure locally, but I can wiht
valgrind enabled.
So, the output of the test is simply telling us that the FSM of the
main table is not getting truncated. From what I can see, the
difference is in should_attempt_truncation(), where we finish with
nonempty_pages set to 1 rather than 0 on failure. And it just takes
one autovacuum to run in parallel of the manual VACUUM after the
DELETE to prevent the removal of those tuples, which is what I can see
from the logs on failure:
LOG: statement: DELETE FROM freespace_tab;
DEBUG: autovacuum: processing database "contrib_regression"
LOG: statement: VACUUM freespace_tab;
It seems to me here that the snapshot hold by autovacuum during the
scan of pg_database to find the relations to process is enough to
prevent the FSM truncation, as the tuples cleaned up by the DELETE
query still need to be visible. One simple way to keep this test
would be a custom configuration file with autovacuum disabled and
NO_INSTALLCHECK. Any better ideas?
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Add pg_freespacemap extension sql test
@ 2022-03-29 05:05 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 17+ messages in thread
From: Michael Paquier @ 2022-03-29 05:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Fabrízio de Royes Mello <fabriziomello@gmail.com>; Dong Wook Lee <sh95119@gmail.com>; pgsql-hackers
On Mon, Mar 28, 2022 at 12:12:48PM +0900, Michael Paquier wrote:
> It seems to me here that the snapshot hold by autovacuum during the
> scan of pg_database to find the relations to process is enough to
> prevent the FSM truncation, as the tuples cleaned up by the DELETE
> query still need to be visible. One simple way to keep this test
> would be a custom configuration file with autovacuum disabled and
> NO_INSTALLCHECK.
Well, done this way. We already do that in other tests that rely on a
FSM truncation to happen, like 008_fsm_truncation.pl.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2022-03-29 05:05 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-03-08 14:39 Add pg_freespacemap extension sql test Dong Wook Lee <sh95119@gmail.com>
2022-03-08 14:43 ` Dong Wook Lee <sh95119@gmail.com>
2022-03-08 16:19 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-03-09 11:13 ` Dong Wook Lee <sh95119@gmail.com>
2022-03-11 05:51 ` Michael Paquier <michael@paquier.xyz>
2022-03-15 13:50 ` Dong Wook Lee <sh95119@gmail.com>
2022-03-19 16:18 ` Dong Wook Lee <sh95119@gmail.com>
2022-03-19 18:13 ` Fabrízio de Royes Mello <fabriziomello@gmail.com>
2022-03-21 12:12 ` Dong Wook Lee <sh95119@gmail.com>
2022-03-23 06:05 ` Michael Paquier <michael@paquier.xyz>
2022-03-23 13:45 ` Fabrízio de Royes Mello <fabriziomello@gmail.com>
2022-03-24 00:40 ` Michael Paquier <michael@paquier.xyz>
2022-03-27 17:18 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-03-28 03:12 ` Michael Paquier <michael@paquier.xyz>
2022-03-29 05:05 ` Michael Paquier <michael@paquier.xyz>
2022-03-08 14:45 ` Justin Pryzby <pryzby@telsasoft.com>
2022-03-08 14:54 ` Dong Wook Lee <sh95119@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox