On 2/9/19, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Feb 5, 2019 at 3:25 PM John Naylor <john.nay...@2ndquadrant.com>
> wrote:
>>
>> On Tue, Feb 5, 2019 at 4:04 AM Amit Kapila <amit.kapil...@gmail.com>
>> wrote:
> This is certainly a good test w.r.t code coverage of new code, but I
> have few comments:
> 1. The size of records in test still depends on alignment (MAXALIGN).
> Though it doesn't seem to be a problematic case, I still suggest we
> can avoid using records whose size depends on alignment. If you
> change the schema as CREATE TABLE fsm_check_size (num1 int, num2 int,
> str text);, then you can avoid alignment related issues for the
> records being used in test.
Done.
> 2.
> +-- Fill most of the last block
> ..
> +-- Make sure records can go into any block but the last one
> ..
> +-- Insert large record and make sure it does not cause the relation to
> extend
>
> The comments in some part of the test seems too focussed towards the
> algorithm used for in-memory map. I think we can keep these if we
> want, but it is required to write a more generic comment stating what
> is the actual motive of additional tests (basically we are testing the
> functionality of in-memory map (LSM) for the heap, so we should write
> about it.).
Done.
> Shall we add a note to the docs of pg_freespacemap and
> pgstattuple_approx indicating that for small relations, FSM won't be
> created, so these functions won't give appropriate value?
I've given this a try in 0002.
> Or other
> possibility could be that we return an error if the block number is
> less than the threshold value, but not sure if that is a good
> alternative as that can happen today also if the vacuum hasn't run on
> the table.
Yeah, an error doesn't seem helpful.
--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 7a7eb0b17aa55dbec2032bf7cc5189e22bb2ca0c Mon Sep 17 00:00:00 2001
From: John Naylor <jcnay...@gmail.com>
Date: Mon, 11 Feb 2019 18:08:01 +0100
Subject: [PATCH v2 1/2] Improve FSM regression test
In the interest of caution, in commit b0eaa4c51bb we left out a
test that used vacuum to remove dead rows. This test has been
rewritten to use fillfactor instead to control free space. Since
we no longer need to remove dead rows as part of the test, put
the fsm regression test in a parallel group.
---
src/test/regress/expected/fsm.out | 36 ++++++++++++++++++++++++------
src/test/regress/parallel_schedule | 8 +------
src/test/regress/sql/fsm.sql | 34 +++++++++++++++++++++++-----
3 files changed, 58 insertions(+), 20 deletions(-)
diff --git a/src/test/regress/expected/fsm.out b/src/test/regress/expected/fsm.out
index b02993188c..f5f62429ac 100644
--- a/src/test/regress/expected/fsm.out
+++ b/src/test/regress/expected/fsm.out
@@ -1,15 +1,37 @@
--
-- Free Space Map test
--
-CREATE TABLE fsm_check_size (num int, str text);
--- With one block, there should be no FSM
-INSERT INTO fsm_check_size VALUES(1, 'a');
+CREATE TABLE fsm_check_size (num1 int, num2 int, str text);
+-- Fill 3 blocks with one record each
+ALTER TABLE fsm_check_size SET (fillfactor=15);
+INSERT INTO fsm_check_size SELECT i, i, rpad('', 1024, 'a')
+FROM generate_series(1,3) i;
+-- There should be no FSM
VACUUM fsm_check_size;
SELECT pg_relation_size('fsm_check_size', 'main') AS heap_size,
pg_relation_size('fsm_check_size', 'fsm') AS fsm_size;
heap_size | fsm_size
-----------+----------
- 8192 | 0
+ 24576 | 0
+(1 row)
+
+-- The following operations are for testing the functionality of the local
+-- in-memory map. In particular, we want to be able to insert into some
+-- other block than the one at the end of the heap, without using a FSM.
+-- Fill most of the last block
+ALTER TABLE fsm_check_size SET (fillfactor=100);
+INSERT INTO fsm_check_size SELECT i, i, rpad('', 1024, 'a')
+FROM generate_series(101,105) i;
+-- Make sure records can go into any block but the last one
+ALTER TABLE fsm_check_size SET (fillfactor=30);
+-- Insert large record and make sure it does not cause the relation to extend
+INSERT INTO fsm_check_size VALUES (111, 111, rpad('', 1024, 'a'));
+VACUUM fsm_check_size;
+SELECT pg_relation_size('fsm_check_size', 'main') AS heap_size,
+pg_relation_size('fsm_check_size', 'fsm') AS fsm_size;
+ heap_size | fsm_size
+-----------+----------
+ 24576 | 0
(1 row)
-- Extend table with enough blocks to exceed the FSM threshold
@@ -19,7 +41,7 @@ num int;
BEGIN
num = 11;
LOOP
- INSERT INTO fsm_check_size VALUES (num, 'b') RETURNING ctid INTO curtid;
+ INSERT INTO fsm_check_size VALUES (num, num, 'b') RETURNING ctid INTO curtid;
EXIT WHEN curtid >= tid '(4, 0)';
num = num + 1;
END LOOP;
@@ -34,8 +56,8 @@ SELECT pg_relation_size('fsm_check_size', 'fsm') AS fsm_size;
-- Add long random string to extend TOAST table to 1 block
INSERT INTO fsm_check_size
-VALUES(0, (SELECT string_agg(md5(chr(i)), '')
- FROM generate_series(1,100) i));
+VALUES(0, 0, (SELECT string_agg(md5(chr(i)), '')
+ FROM generate_series(1,100) i));
VACUUM fsm_check_size;
SELECT pg_relation_size(reltoastrelid, 'main') AS toast_size,
pg_relation_size(reltoastrelid, 'fsm') AS toast_fsm_size
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 4051a4ad4e..a956775dd1 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -23,7 +23,7 @@ test: numerology
# ----------
# The second group of parallel tests
# ----------
-test: point lseg line box path polygon circle date time timetz timestamp timestamptz interval inet macaddr macaddr8 tstypes
+test: point lseg line box path polygon circle date time timetz timestamp timestamptz interval inet macaddr macaddr8 tstypes fsm
# ----------
# Another group of parallel tests
@@ -68,12 +68,6 @@ test: create_aggregate create_function_3 create_cast constraints triggers inheri
# ----------
test: sanity_check
-# ----------
-# fsm does a delete followed by vacuum, and running it in parallel can prevent
-# removal of rows.
-# ----------
-test: fsm
-
# ----------
# Believe it or not, select creates a table, subsequent
# tests need.
diff --git a/src/test/regress/sql/fsm.sql b/src/test/regress/sql/fsm.sql
index 332c3e2b2d..7ba22e5bd1 100644
--- a/src/test/regress/sql/fsm.sql
+++ b/src/test/regress/sql/fsm.sql
@@ -2,10 +2,32 @@
-- Free Space Map test
--
-CREATE TABLE fsm_check_size (num int, str text);
+CREATE TABLE fsm_check_size (num1 int, num2 int, str text);
--- With one block, there should be no FSM
-INSERT INTO fsm_check_size VALUES(1, 'a');
+-- Fill 3 blocks with one record each
+ALTER TABLE fsm_check_size SET (fillfactor=15);
+INSERT INTO fsm_check_size SELECT i, i, rpad('', 1024, 'a')
+FROM generate_series(1,3) i;
+
+-- There should be no FSM
+VACUUM fsm_check_size;
+SELECT pg_relation_size('fsm_check_size', 'main') AS heap_size,
+pg_relation_size('fsm_check_size', 'fsm') AS fsm_size;
+
+-- The following operations are for testing the functionality of the local
+-- in-memory map. In particular, we want to be able to insert into some
+-- other block than the one at the end of the heap, without using a FSM.
+
+-- Fill most of the last block
+ALTER TABLE fsm_check_size SET (fillfactor=100);
+INSERT INTO fsm_check_size SELECT i, i, rpad('', 1024, 'a')
+FROM generate_series(101,105) i;
+
+-- Make sure records can go into any block but the last one
+ALTER TABLE fsm_check_size SET (fillfactor=30);
+
+-- Insert large record and make sure it does not cause the relation to extend
+INSERT INTO fsm_check_size VALUES (111, 111, rpad('', 1024, 'a'));
VACUUM fsm_check_size;
SELECT pg_relation_size('fsm_check_size', 'main') AS heap_size,
@@ -18,7 +40,7 @@ num int;
BEGIN
num = 11;
LOOP
- INSERT INTO fsm_check_size VALUES (num, 'b') RETURNING ctid INTO curtid;
+ INSERT INTO fsm_check_size VALUES (num, num, 'b') RETURNING ctid INTO curtid;
EXIT WHEN curtid >= tid '(4, 0)';
num = num + 1;
END LOOP;
@@ -30,8 +52,8 @@ SELECT pg_relation_size('fsm_check_size', 'fsm') AS fsm_size;
-- Add long random string to extend TOAST table to 1 block
INSERT INTO fsm_check_size
-VALUES(0, (SELECT string_agg(md5(chr(i)), '')
- FROM generate_series(1,100) i));
+VALUES(0, 0, (SELECT string_agg(md5(chr(i)), '')
+ FROM generate_series(1,100) i));
VACUUM fsm_check_size;
SELECT pg_relation_size(reltoastrelid, 'main') AS toast_size,
--
2.17.1
From e6e253064324dabedbc06f145c4edce9be5d54b4 Mon Sep 17 00:00:00 2001
From: John Naylor <jcnay...@gmail.com>
Date: Mon, 11 Feb 2019 18:09:00 +0100
Subject: [PATCH v2 2/2] Document that functions that use the FSM for will
report no free space for tables without a FSM.
---
contrib/pgstattuple/pgstatapprox.c | 2 ++
doc/src/sgml/pgfreespacemap.sgml | 2 ++
doc/src/sgml/pgstattuple.sgml | 4 +++-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index ff7c255a2d..23ccaaaca6 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -89,6 +89,8 @@ statapprox_heap(Relation rel, output_type *stat)
/*
* If the page has only visible tuples, then we can find out the free
* space from the FSM and move on.
+ * Note: If a relation has no FSM, GetRecordedFreeSpace() will report
+ * zero free space. This is fine for the purposes of approximation.
*/
if (VM_ALL_VISIBLE(rel, blkno, &vmbuffer))
{
diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml
index 0122d278e3..be6fcc4986 100644
--- a/doc/src/sgml/pgfreespacemap.sgml
+++ b/doc/src/sgml/pgfreespacemap.sgml
@@ -61,6 +61,8 @@
The values stored in the free space map are not exact. They're rounded
to precision of 1/256th of <symbol>BLCKSZ</symbol> (32 bytes with default <symbol>BLCKSZ</symbol>), and
they're not kept fully up-to-date as tuples are inserted and updated.
+ In addition, small tables don't have a free space map, so this function
+ will return zero even if free space is available.
</para>
<para>
diff --git a/doc/src/sgml/pgstattuple.sgml b/doc/src/sgml/pgstattuple.sgml
index b17b3c59e0..8c5f53674d 100644
--- a/doc/src/sgml/pgstattuple.sgml
+++ b/doc/src/sgml/pgstattuple.sgml
@@ -527,7 +527,9 @@ approx_free_percent | 2.09
bit set, then it is assumed to contain no dead tuples). For such
pages, it derives the free space value from the free space map, and
assumes that the rest of the space on the page is taken up by live
- tuples.
+ tuples. Small tables don't have a free space map, so in that case
+ this function will report zero free space, likewise inflating the
+ estimated number of live tuples.
</para>
<para>
--
2.17.1
