Re: Pluggable toaster

Поиск
Список
Период
Сортировка
От Nikita Malakhov
Тема Re: Pluggable toaster
Дата
Msg-id CAN-LCVPVKX75bAi_wVTOdDCPFpJcDsAfPAFVCNUrR=OmV-_ZuQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Pluggable toaster  (Aleksander Alekseev <aleksander@timescale.com>)
Список pgsql-hackers
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 

On Fri, Jul 1, 2022 at 11:10 AM Aleksander Alekseev <aleksander@timescale.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


В списке pgsql-hackers по дате отправления:

Предыдущее
От: Robert Haas
Дата:
Сообщение: Re: replacing role-level NOINHERIT with a grant-level option
Следующее
От: Joe Conway
Дата:
Сообщение: Re: replacing role-level NOINHERIT with a grant-level option