Message-ID: <a595a888-1dde-42a0-becd-c9c313ac781a@enterprisedb.com>
Date: Sun, 3 Mar 2024 21:29:42 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Shared detoast Datum proposal
Content-Language: en-US
To: Andy Fan <zhihuifan1213@163.com>
Cc: Nikita Malakhov <hukutoc@gmail.com>,
 Michael Zhilin <m.zhilin@postgrespro.ru>, Peter Smith
 <smithpb2250@gmail.com>, vignesh C <vignesh21@gmail.com>,
 Matthias van de Meent <boekewurm+postgres@gmail.com>,
 pgsql-hackers@lists.postgresql.org
References: <87il4jrk1l.fsf@163.com> <87h6jxw41k.fsf@163.com>
 <CALDaNm2EtgXGd+w81ZjTZBD_rLSURyGqXRoz7EFfhxEneOmWig@mail.gmail.com>
 <87sf3andy2.fsf@163.com> <87wmskm9qu.fsf@163.com>
 <CAHut+PsehMtwQ4uE9VS2C7jB1yT0SZVOPU22u1=ma1K7dKFUow@mail.gmail.com>
 <87v87kvboq.fsf@163.com>
 <71321d83-391b-473b-aaf6-618c7f682b96@postgrespro.ru>
 <87r0i7vos3.fsf@163.com> <874je3hcwn.fsf@163.com>
 <c066e988-38b9-460f-9e9d-aedd2482bff2@enterprisedb.com>
 <8734tmg88e.fsf@163.com>
 <7f4c8673-4c5d-4dbf-b8e5-0ec11cd2fba0@enterprisedb.com>
 <87h6i2djf9.fsf@163.com> <87jzms0xza.fsf@163.com>
 <CAN-LCVMU_AzYNDQ-MgX3xc5qCnhTgO1LWKUCygOu3neBM-y94g@mail.gmail.com>
 <87frxf1gld.fsf@163.com>
 <fea81484-09b5-499d-80d0-acb8df43d047@enterprisedb.com>
 <875xyb1a6q.fsf@163.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <875xyb1a6q.fsf@163.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/a595a888-1dde-42a0-becd-c9c313ac781a%40enterprisedb.com>
Precedence: bulk


On 2/26/24 16:29, Andy Fan wrote:
>
> ...>
> I can understand the benefits of the TOAST cache, but the following
> issues looks not good to me IIUC: 
> 
> 1). we can't put the result to tts_values[*] since without the planner
> decision, we don't know if this will break small_tlist logic. But if we
> put it into the cache only, which means a cache-lookup as a overhead is
> unavoidable.

True - if you're comparing having the detoasted value in tts_values[*]
directly with having to do a lookup in a cache, then yes, there's a bit
of an overhead.

But I think from the discussion it's clear having to detoast the value
into tts_values[*] has some weaknesses too, in particular:

- It requires decisions which attributes to detoast eagerly, which is
quite invasive (having to walk the plan, ...).

- I'm sure there will be cases where we choose to not detoast, but it
would be beneficial to detoast.

- Detoasting just the initial slices does not seem compatible with this.

IMHO the overhead of the cache lookup would be negligible compared to
the repeated detoasting of the value (which is the current baseline). I
somewhat doubt the difference compared to tts_values[*] will be even
measurable.

> 
> 2). It is hard to decide which entry should be evicted without attaching
> it to the TupleTableSlot's life-cycle. then we can't grantee the entry
> we keep is the entry we will reuse soon?
> 

True. But is that really a problem? I imagined we'd set some sort of
memory limit on the cache (work_mem?), and evict oldest entries. So the
entries would eventually get evicted, and the memory limit would ensure
we don't consume arbitrary amounts of memory.

We could also add some "slot callback" but that seems unnecessary.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company