On 12.01.2021 00:51, Tomas Vondra wrote:
On 1/11/21 10:00 PM, Anastasia Lubennikova wrote:
On 11.01.2021 01:35, Tomas Vondra wrote:
Hi,
I started looking at this patch again, hoping to get it committed in
this CF, but I think there's a regression in handling TOAST tables
(compared to the v3 patch submitted by Pavan in February 2019).
The test I'm running a very simple test (see test.sql):
1) start a transaction
2) create a table with a text column
3) copy freeze data into it
4) use pg_visibility to see how many blocks are all_visible both in the
main table and it's TOAST table
For v3 patch (applied on top of 278584b526 and
s/hi_options/ti_options) I get this:
pages NOT all_visible
------------------------------------------
main 637 0
toast 50001 3
There was some discussion about relcache invalidations causing a
couple TOAST pages not be marked as all_visible, etc.
However, for this patch on master I get this
pages NOT all_visible
------------------------------------------
main 637 0
toast 50001 50001
So no pages in TOAST are marked as all_visible. I haven't
investigated what's causing this, but IMO that needs fixing to make
ths patch RFC.
Attached is the test script I'm using, and a v5 of the patch -
rebased on current master, with some minor tweaks to comments etc.
Thank you for attaching the test script. I reproduced the problem.
This regression occurs because TOAST internally uses heap_insert().
You have asked upthread about adding this optimization to heap_insert().
I wrote a quick fix, see the attached patch 0002. The TOAST test
passes now, but I haven't tested performance or any other use-cases yet.
I'm going to test it properly in a couple of days and share results.
Thanks. I think it's important to make this work for TOAST tables - it
often stores most of the data, and it was working in v3 of the patch.
I haven't looked into the details, but if it's really just due to
TOAST using heap_insert, I'd say it just confirms the importance of
tweaking heap_insert too.
I've tested performance. All tests were run on my laptop, latest master
with and without patches, all default settings, except disabled
autovacuum and installed pg_stat_statements extension.
The VACUUM is significantly faster with the patch, as it only checks
visibility map. COPY speed fluctuates a lot between tests, but I didn't
notice any trend.
I would expect minor slowdown with the patch, as we need to handle
visibility map pages during COPY FREEZE. But in some runs, patched
version was faster than current master, so the impact of the patch is
insignificant.
I run 3 different tests:
1) Regular table (final size 5972 MB)
patched | master
COPY FREEZE data 3 times:
33384,544 ms 31135,037 ms
31666,226 ms 31158,294 ms
32802,783 ms 33599,234 ms
VACUUM
54,185 ms 48445,584 ms
2) Table with TOAST (final size 1207 MB where 1172 MB is in toast table)
patched | master
COPY FREEZE data 3 times:
368166,743 ms 383231,077 ms
398695,018 ms 454572,630 ms
410174,682 ms 567847,288 ms
VACUUM
43,159 ms 6648,302 ms
3) Table with a trivial BEFORE INSERT trigger (final size 5972 MB)
patched | master
COPY FREEZE data 3 times:
73544,225 ms 64967,802 ms
90960,584 ms 71826,362 ms
81356,025 ms 80023,041 ms
VACUUM
49,626 ms 40100,097 ms
I took another look at the yesterday's patch and it looks fine to me. So
now I am waiting for your review.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company