From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: lubennikovaav@gmail.com
Cc: pgsql-hackers@lists.postgresql.org
Cc: pavan.deolasee@gmail.com
Cc: ibrar.ahmad@gmail.com
Subject: Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits
Date: Mon, 11 Jan 2021 22:51:21 +0100
Message-ID: <f6e0af44-b147-d27f-cc21-2404b4f11f0e@enterprisedb.com> (raw)
In-Reply-To: <75f3760f-1077-07c1-aea4-975d5c127d6d@postgrespro.ru>
References: <CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com>
<CALtqXTfpEOiyJchYQoUKw9pvEPhVgnkxL=JvKNj57BTxva8bTA@mail.gmail.com>
<160382617166.1204.475752573208579406.pgcf@coridan.postgresql.org>
<20201028.144653.966121790856478893.t-ishii@sraoss.co.jp>
<20201030004252.d63rwclys4bniluu@development>
<1d2e0f4a-6ab8-10f1-c177-af97475df001@postgrespro.ru>
<20201102144453.tumlk7qhw3gzn2dk@development>
<7e6a006f-c59a-93fc-3ddf-f9c661319bc8@2ndquadrant.com>
<75f3760f-1077-07c1-aea4-975d5c127d6d@postgrespro.ru>
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.
> With this change a lot of new code is repeated in heap_insert() and
> heap_multi_insert(). I think it's fine, because these functions already
> have a lot in common.
>
Understood. IMHO a bit of redundancy is not a big issue, but I haven't
looked at the code yet. Let's get it working first, then we can decide
if some refactoring is appropriate.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@enterprisedb.com, a.lubennikova@postgrespro.ru, tomas.vondra@2ndquadrant.com, ishii@sraoss.co.jp, lubennikovaav@gmail.com, pgsql-hackers@lists.postgresql.org, pavan.deolasee@gmail.com, ibrar.ahmad@gmail.com
Subject: Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits
In-Reply-To: <f6e0af44-b147-d27f-cc21-2404b4f11f0e@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox