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

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es

From: Amit Kapila <amit.kapila16@gmail.com>

To: John Naylor <john.naylor@2ndquadrant.com>

Cc: pgsql-hackers <pgsql-hackers@postgresql.org>

Date: 2019-02-20T11:09:09Z

Lists: pgsql-hackers

Attachments

v3-0002-Document-that-functions-that-use-the-FSM-for-will.patch (application/octet-stream) patch v3-0002

On Mon, Feb 11, 2019 at 10:48 PM John Naylor
<john.naylor@2ndquadrant.com> wrote:
>
> On 2/9/19, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > 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.
>

This looks mostly correct, but I have a few observations:
1.
-      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.

The last part of the sentence "likewise inflating the estimated number
of live tuples." seems incorrect to me because live tuples are
computed based on the pages scanned, live tuples in them and total
blocks in the relation.  So, I think it should be "likewise inflating
the approximate tuple length".

2.
+   In addition, small tables don't have a free space map, so this function
+   will return zero even if free space is available.

Actually, the paragraph you have modified applies to both the
functions mentioned on that page.  So instead of saying "this function
..", we can say "these functions .."

3.
* 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.
  */

It is better to have an empty line before Note: ...

I have modified the patch for the above observations and added a
commit message as well, see if it looks okay to you.

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

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