Re: WIP: Avoid creation of the free space map for small tables
John Naylor <jcnaylor@gmail.com>
From: John Naylor <jcnaylor@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-11-29T09:37:30Z
Lists: pgsql-hackers
Attachments
- v9-0001-Avoid-creation-of-the-free-space-map-for-small-ta.patch (text/x-patch) patch v9-0001
- v9-0002-During-pg_upgrade-skip-transfer-of-FSMs-if-they-w.patch (text/x-patch) patch v9-0002
> Is the point 3 change related to pgindent? I think even if you want > these, then don't prepare other patches on top of this, keep it > entirely separate. Both removed. >> Also, we don't quite have a consensus on the threshold value, but I >> have set it to 4 pages for v8. If this is still considered too >> expensive (and basic tests show it shouldn't be), I suspect it'd be >> better to interleave the available block numbers as described a couple >> days ago than lower the threshold further. >> > > Can you please repeat the copy test you have done above with > fillfactor as 20 and 30? I will send the results in a separate email soon. > Few more comments: > ------------------------------- > 1. I think we can add some test(s) to test the new functionality, may > be something on the lines of what Robert has originally provided as an > example of this behavior [1]. Done. I tried adding it to several schedules, but for some reason vacuuming an empty table failed to truncate the heap to 0 blocks. Putting the test in its own group fixed the problem, but that doesn't seem ideal. > 2. > The similar call is required in AbortSubTransaction function as well. > I suggest to add it after pgstat_progress_end_command in both > functions. Done. > 3. >> agree we shouldn't change that without a reason to. One simple idea is >> add a 3rd boolean parameter to GetPageWithFreeSpace() to control >> whether it gives up if the FSM fork doesn't indicate free space, like > I have the exact fix in my mind, so let's do it that way. Done. This also reverts comments and variable names that referred to updating the local map after relation extension. While at it, I changed a couple conditionals to check the locally cached nblocks rather than the threshold. No functional change, but looks more precise. Might save a few cycles as well. >> > 5. Your logic to update FSM on standby seems okay, but can you show >> > some tests which proves its sanity? >> >> I believe to convince myself it was working, I used the individual >> commands in the sql file in [3], then used the size function on the >> secondary. I'll redo that to verify. I've verified the standby behaves precisely as the primary, as far as the aforementioned script goes. -John Naylor
Commits
-
Improve code comments in b0eaa4c51b.
- 06c8a5090ed9 12.0 landed
-
During pg_upgrade, conditionally skip transfer of FSMs.
- 13e8643bfc29 12.0 landed
-
Add more tests for FSM.
- 6f918159a97a 12.0 landed
-
Doc: Update the documentation for FSM behavior for small tables.
- 29d108cdecbe 12.0 landed
-
Make FSM test portable.
- 08ecdfe7e5e0 12.0 landed
-
Avoid creation of the free space map for small heap relations, take 2.
- b0eaa4c51bbf 12.0 landed
-
Move page initialization from RelationAddExtraBlocks() to use, take 2.
- 0d1fe9f74e36 12.0 cited
-
Avoid creation of the free space map for small heap relations.
- ac88d2962a96 12.0 landed
-
In bootstrap mode, don't allow the creation of files if they don't already
- d66e3664b8ba 12.0 landed