Re: Pluggable toaster

Hi,
Alexander, thank you for your feedback!
I'd explain our thoughts:
We thought that refactoring default TOAST mechanics via TOAST API in p.
0002 would be too much because the API itself already
introduced a lot of changes, so we kept Default Toasters re-implementation
for later patch.
0002 introduces custom Toast pointers with corresponding macro set
(postgres.h), Dummy toaster as an example for developers
how the API should be used, but left default TOAST as-is. As I see, there
are no lots of custom TAMs, despite of pluggable storage
interface introduced several years ago, so we thought that some simple
example of how to use the new API would be nice to have.
We've done TOAST refactoring in 0003, but it has not replaced default
TOAST, it just routed it default via TOAST API, but it still
is left as part of the core, and is used for TOASTing (set
in atttoaster column) by default.

For performance testing we used a lot of manually corrected scripts, I have
to put them in order but I would provide them later as
additional side patch for this patch set.

Best regards,
--
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/

On Fri, Jul 1, 2022 at 11:10 AM Aleksander Alekseev <
aleksander(at)timescale(dot)com> wrote:

> Hi Nikita,
>
> > Here is the patch set rebased onto current master (15 rel beta 2 with
> commit from 29.06).
>
> Thanks for the rebased patchset.
>
> This is a huge effort though. I suggest splitting it into several CF
> entries as we previously did with other patches (64-bit XIDs to name
> one, which BTW is arguably much simpler, but still we had to split
> it). This will simplify the review, limit the scope of the discussion
> and simplify passing the CI. Cfbot is already not happy with the
> patchset.
>
> 0001 - is already in a separate thread [1], that's good. I suggest
> marking it in the patch description for clarity.
> 0002, 0003 - I suggest focusing on these two in this thread and keep
> the rest of the changes for later discussion. Please submit 0004,
> 0005... next time, when we finish with 0001-0003.
>
> The order of proposed changes IMO is wrong.
>
> 0002 should refactor the default TOASTer in a manner similar to a
> pluggable one. Nothing should change from the user's perspective. If
> you do benchmarks, I suggest not to reference the previous talks. I
> familiarized myself with all the related talks linked before (took me
> some time...) and found them useless for the discussion since they
> don't provide exact steps to reproduce. Please provide exact scripts
> and benchmarks results for 0002 in this thread.
>
> 0003 should add an interface that allows replacing the default TOASTer
> with an alternative one. There is no need for contrib/dummy_toaster
> similarly as there is no contrib/ for a dummy TableAM. The provided
> example doesn't do much anyway since all the heavy lifting should be
> done in the callbacks implementation. For test purposes please use
> src/test/regress/.
>
> [1]:
> https://www.postgresql.org/message-id/de83407a-ae3d-a8e1-a788-920eb334f25b%40sigaev.ru
>
> --
> Best regards,
> Aleksander Alekseev
>