Re: WIP: Avoid creation of the free space map for small tables

On Thu, Jan 17, 2019 at 11:13 PM John Naylor
<john.naylor@2ndquadrant.com> wrote:
>
> On Wed, Jan 16, 2019 at 10:35 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > Yes, I think it would be good if you can explain the concept of
> > local-map with the help of this example.
>
> > Then let's not add a reference to the version number in this case.  I
>
> Okay, done in v14. I kept your spelling of the new macro. One minor
> detail added: use uint8 rather than char for the local map array. This
> seems to be preferred, especially in this file.
>

I am fine with your change.

Few more comments:
1.
I think we should not allow to create FSM for toast tables as well
till there size reaches HEAP_FSM_CREATION_THRESHOLD.  If you try below
test, you can see that FSM will be created for the toast table even if
the size of toast relation is 1 page.

CREATE OR REPLACE FUNCTION random_text(length INTEGER)
RETURNS TEXT
LANGUAGE SQL
 AS $$ select string_agg(chr
(32+(random()*96)::int), '') from generate_series(1,length); $$;

create table tt(c1 int, c2 text);
insert into tt values(1, random_text(2500));
Vacuum tt;

I have fixed this in the attached patch, kindly verify it once and see
if you can add the test for same as well.

2.
-CREATE TABLE test1 (a int, b int);
-INSERT INTO test1 VALUES (16777217, 131584);
+CREATE TABLE test_rel_forks (a
int);
+-- Make sure there are enough blocks in the heap for the FSM to be created.
+INSERT INTO test_rel_forks SELECT g
from generate_series(1,10000) g;

-VACUUM test1;  -- set up FSM
+-- set up FSM and VM
+VACUUM test_rel_forks;

This test will create 45 pages instead of 1.  I know that to create
FSM, we now need more than 4 pages, but 45 seems to be on the higher
side.  I think we should not unnecessarily populate more data if there
is no particular need for it, let's restrict the number of pages to 5
if possible.

3.
-SELECT octet_length(get_raw_page('test1', 'fsm', 1)) AS fsm_1;
- fsm_1
--------
-  8192
-(1 row)
-
-SELECT octet_length
(get_raw_page('test1', 'vm', 0)) AS vm_0;
+SELECT octet_length(get_raw_page('test_rel_forks', 'fsm', 10)) AS fsm_10;
+ERROR:  block number 10 is out of range for relation "test_rel_forks"

Why have you changed the test definition here?  Previously test checks
the existing FSM page, but now it tries to access out of range page.

Apart from the above, I have changed one sentence in README.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com