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

> 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

  1. Improve code comments in b0eaa4c51b.

  2. During pg_upgrade, conditionally skip transfer of FSMs.

  3. Add more tests for FSM.

  4. Doc: Update the documentation for FSM behavior for small tables.

  5. Make FSM test portable.

  6. Avoid creation of the free space map for small heap relations, take 2.

  7. Move page initialization from RelationAddExtraBlocks() to use, take 2.

  8. Avoid creation of the free space map for small heap relations.

  9. In bootstrap mode, don't allow the creation of files if they don't already