public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v3] Move page initialization from RelationAddExtraBlocks() to use, take 2.
39+ messages / 7 participants
[nested] [flat]
* [PATCH v3] Move page initialization from RelationAddExtraBlocks() to use, take 2.
@ 2019-02-01 15:08 Andres Freund <[email protected]>
0 siblings, 0 replies; 39+ messages in thread
From: Andres Freund @ 2019-02-01 15:08 UTC (permalink / raw)
Previously we initialized pages when bulk extending in
RelationAddExtraBlocks(). That has a major disadvantage: It ties
RelationAddExtraBlocks() to heap, as other types of storage are likely
to need different amounts of special space, have different amount of
free space (previously determined by PageGetHeapFreeSpace()).
That we're relying on initializing pages, but not WAL logging the
initialization, also means the risk for getting
"WARNING: relation \"%s\" page %u is uninitialized --- fixing"
style warnings in vacuums after crashes/immediate shutdowns, is
considerably higher. The warning sounds much more serious than what
they are.
Fix those two issues together by not initializing pages in
RelationAddExtraPages() (but continue to do so in
RelationGetBufferForTuple(), which is linked much more closely to
heap), and accepting uninitialized pages as normal in
vacuumlazy.c. When vacuumlazy encounters an empty page it now adds it
to the FSM, but does nothing else. We chose to not issue a debug
message, much less a warning in that case - it seems rarely useful,
and quite likely to scare people unnecessarily.
For now empty pages aren't added to the VM, because standbys would not
re-discover such pages after a promotion. In contrast to other sources
for empty pages, there's no corresponding WAL records triggering FSM
updates during replay.
Previously when extending the relation, there was a moment between
extending the relation, and acquiring an exclusive lock on the new
page, in which another backend could lock the page. To avoid new
content being put on that new page, vacuumlazy needed to acquire the
extension lock for a brief moment when encountering a new page. A
second corner case, only working somewhat by accident, was that
RelationGetBufferForTuple() sometimes checks the last page in a
relation for free space, without consulting the fsm; that only worked
because PageGetHeapFreeSpace() interprets the zero page header in a
new page as no free space. The lack of handling this properly
required reverting the previous attempt in 684200543b.
This issue can be solved by using RBM_ZERO_AND_LOCK when extending the
relation, thereby avoiding this window. There's some added complexity
when RelationGetBufferForTuple() is called with another buffer (for
updates), to avoid deadlocks.
Author: Andres Freund
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/heap/hio.c | 120 ++++++++++++++++++---------
src/backend/access/heap/vacuumlazy.c | 84 ++++++++++---------
2 files changed, 127 insertions(+), 77 deletions(-)
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 3da0b49ccc4..5a108b7fe66 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -74,23 +74,31 @@ RelationPutHeapTuple(Relation relation,
}
/*
- * Read in a buffer, using bulk-insert strategy if bistate isn't NULL.
+ * Read in a buffer in mode, using bulk-insert strategy if bistate isn't NULL.
*/
static Buffer
ReadBufferBI(Relation relation, BlockNumber targetBlock,
- BulkInsertState bistate)
+ ReadBufferMode mode, BulkInsertState bistate)
{
Buffer buffer;
/* If not bulk-insert, exactly like ReadBuffer */
if (!bistate)
- return ReadBuffer(relation, targetBlock);
+ return ReadBufferExtended(relation, MAIN_FORKNUM, targetBlock,
+ mode, NULL);
/* If we have the desired block already pinned, re-pin and return it */
if (bistate->current_buf != InvalidBuffer)
{
if (BufferGetBlockNumber(bistate->current_buf) == targetBlock)
{
+ /*
+ * Currently the LOCK variants are only used for extending
+ * relation, which should never reach this branch.
+ */
+ Assert(mode != RBM_ZERO_AND_LOCK &&
+ mode != RBM_ZERO_AND_CLEANUP_LOCK);
+
IncrBufferRefCount(bistate->current_buf);
return bistate->current_buf;
}
@@ -101,7 +109,7 @@ ReadBufferBI(Relation relation, BlockNumber targetBlock,
/* Perform a read using the buffer strategy */
buffer = ReadBufferExtended(relation, MAIN_FORKNUM, targetBlock,
- RBM_NORMAL, bistate->strategy);
+ mode, bistate->strategy);
/* Save the selected block as target for future inserts */
IncrBufferRefCount(buffer);
@@ -204,11 +212,10 @@ RelationAddExtraBlocks(Relation relation, BulkInsertState bistate)
/*
* Extend by one page. This should generally match the main-line
* extension code in RelationGetBufferForTuple, except that we hold
- * the relation extension lock throughout.
+ * the relation extension lock throughout, and we don't immediately
+ * initialize the page (see below).
*/
- buffer = ReadBufferBI(relation, P_NEW, bistate);
-
- LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+ buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
page = BufferGetPage(buffer);
if (!PageIsNew(page))
@@ -216,18 +223,18 @@ RelationAddExtraBlocks(Relation relation, BulkInsertState bistate)
BufferGetBlockNumber(buffer),
RelationGetRelationName(relation));
- PageInit(page, BufferGetPageSize(buffer), 0);
-
/*
- * We mark all the new buffers dirty, but do nothing to write them
- * out; they'll probably get used soon, and even if they are not, a
- * crash will leave an okay all-zeroes page on disk.
+ * Add the page to the FSM without initializing. If we were to
+ * initialize here the page would potentially get flushed out to disk
+ * before we add any useful content. There's no guarantee that that'd
+ * happen before a potential crash, so we need to deal with
+ * uninitialized pages anyway, thus avoid the potential for
+ * unnecessary writes.
*/
- MarkBufferDirty(buffer);
/* we'll need this info below */
blockNum = BufferGetBlockNumber(buffer);
- freespace = PageGetHeapFreeSpace(page);
+ freespace = BufferGetPageSize(buffer) - SizeOfPageHeaderData;
UnlockReleaseBuffer(buffer);
@@ -412,7 +419,7 @@ loop:
if (otherBuffer == InvalidBuffer)
{
/* easy case */
- buffer = ReadBufferBI(relation, targetBlock, bistate);
+ buffer = ReadBufferBI(relation, targetBlock, RBM_NORMAL, bistate);
if (PageIsAllVisible(BufferGetPage(buffer)))
visibilitymap_pin(relation, targetBlock, vmbuffer);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
@@ -479,6 +486,18 @@ loop:
* we're done.
*/
page = BufferGetPage(buffer);
+
+ /*
+ * Initialize page, it'll be used soon. We could avoid dirtying the
+ * buffer here, and rely on the caller to do so whenever it puts a
+ * tuple onto the page, but there seems not much benefit in doing so.
+ */
+ if (PageIsNew(page))
+ {
+ PageInit(page, BufferGetPageSize(buffer), 0);
+ MarkBufferDirty(buffer);
+ }
+
pageFreeSpace = PageGetHeapFreeSpace(page);
if (len + saveFreeSpace <= pageFreeSpace)
{
@@ -571,28 +590,7 @@ loop:
* it worth keeping an accurate file length in shared memory someplace,
* rather than relying on the kernel to do it for us?
*/
- buffer = ReadBufferBI(relation, P_NEW, bistate);
-
- /*
- * We can be certain that locking the otherBuffer first is OK, since it
- * must have a lower page number.
- */
- if (otherBuffer != InvalidBuffer)
- LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
-
- /*
- * Now acquire lock on the new page.
- */
- LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
-
- /*
- * Release the file-extension lock; it's now OK for someone else to extend
- * the relation some more. Note that we cannot release this lock before
- * we have buffer lock on the new page, or we risk a race condition
- * against vacuumlazy.c --- see comments therein.
- */
- if (needLock)
- UnlockRelationForExtension(relation, ExclusiveLock);
+ buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
/*
* We need to initialize the empty new page. Double-check that it really
@@ -607,6 +605,52 @@ loop:
RelationGetRelationName(relation));
PageInit(page, BufferGetPageSize(buffer), 0);
+ MarkBufferDirty(buffer);
+
+ /*
+ * Release the file-extension lock; it's now OK for someone else to extend
+ * the relation some more.
+ */
+ if (needLock)
+ UnlockRelationForExtension(relation, ExclusiveLock);
+
+ /*
+ * Lock the other buffer. It's guaranteed to be of a lower page number
+ * than the new page. To conform with the deadlock prevent rules, we ought
+ * to lock otherBuffer first, but that would give other backends a chance
+ * to put tuples on our page. To reduce the likelihood of that, attempt to
+ * lock the other buffer conditionally, that's very likely to work.
+ * Otherwise we need to lock buffers in the correct order, and retry if
+ * the space has been used in the mean time.
+ *
+ * Alternatively, we could acquire the lock on otherBuffer before
+ * extending the relation, but that'd require holding the lock while
+ * performing IO, which seems worse than an unlikely retry.
+ */
+ if (otherBuffer != InvalidBuffer)
+ {
+ Assert(otherBuffer != buffer);
+
+ if (unlikely(!ConditionalLockBuffer(otherBuffer)))
+ {
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+ LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
+ LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+
+ /*
+ * Because the buffer was unlocked for a while, it's possible,
+ * although unlikely, that the page was filled. If so, just retry
+ * from start.
+ */
+ if (len > PageGetHeapFreeSpace(page))
+ {
+ LockBuffer(otherBuffer, BUFFER_LOCK_UNLOCK);
+ UnlockReleaseBuffer(buffer);
+
+ goto loop;
+ }
+ }
+ }
if (len > PageGetHeapFreeSpace(page))
{
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 37aa484ec3a..26dfb0c7e0f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -860,43 +860,46 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
if (PageIsNew(page))
{
+ bool still_new;
+
/*
- * An all-zeroes page could be left over if a backend extends the
- * relation but crashes before initializing the page. Reclaim such
- * pages for use.
+ * All-zeroes pages can be left over if either a backend extends
+ * the relation by a single page, but crashes before the newly
+ * initialized page has been written out, or when bulk-extending
+ * the relation (which creates a number of empty pages at the tail
+ * end of the relation, but enters them into the FSM).
*
- * We have to be careful here because we could be looking at a
- * page that someone has just added to the relation and not yet
- * been able to initialize (see RelationGetBufferForTuple). To
- * protect against that, release the buffer lock, grab the
- * relation extension lock momentarily, and re-lock the buffer. If
- * the page is still uninitialized by then, it must be left over
- * from a crashed backend, and we can initialize it.
+ * Make sure these pages are in the FSM, to ensure they can be
+ * reused. Do that by testing if there's any space recorded for
+ * the page. If not, enter it.
*
- * We don't really need the relation lock when this is a new or
- * temp relation, but it's probably not worth the code space to
- * check that, since this surely isn't a critical path.
- *
- * Note: the comparable code in vacuum.c need not worry because
- * it's got exclusive lock on the whole relation.
+ * Note we do not enter the page into the visibilitymap. That has
+ * the downside that we repeatedly visit this page in subsequent
+ * vacuums, but otherwise we'll never not discover the space on a
+ * promoted standby. The harm of repeated checking ought to
+ * normally not be too bad - the space usually should be used at
+ * some point, otherwise there wouldn't be any regular vacuums.
*/
- LockBuffer(buf, BUFFER_LOCK_UNLOCK);
- LockRelationForExtension(onerel, ExclusiveLock);
- UnlockRelationForExtension(onerel, ExclusiveLock);
- LockBufferForCleanup(buf);
- if (PageIsNew(page))
- {
- ereport(WARNING,
- (errmsg("relation \"%s\" page %u is uninitialized --- fixing",
- relname, blkno)));
- PageInit(page, BufferGetPageSize(buf), 0);
- empty_pages++;
- }
- freespace = PageGetHeapFreeSpace(page);
- MarkBufferDirty(buf);
+
+ /*
+ * Perform checking of FSM after releasing lock, the fsm is
+ * approximate, after all.
+ */
+ still_new = PageIsNew(page);
UnlockReleaseBuffer(buf);
- RecordPageWithFreeSpace(onerel, blkno, freespace);
+ if (still_new)
+ {
+ empty_pages++;
+
+ if (GetRecordedFreeSpace(onerel, blkno) == 0)
+ {
+ Size freespace;
+
+ freespace = BufferGetPageSize(buf) - SizeOfPageHeaderData;
+ RecordPageWithFreeSpace(onerel, blkno, freespace);
+ }
+ }
continue;
}
@@ -905,7 +908,10 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
empty_pages++;
freespace = PageGetHeapFreeSpace(page);
- /* empty pages are always all-visible and all-frozen */
+ /*
+ * Empty pages are always all-visible and all-frozen (note that
+ * the same is currently not true for new pages, see above).
+ */
if (!PageIsAllVisible(page))
{
START_CRIT_SECTION();
@@ -1639,12 +1645,13 @@ lazy_check_needs_freeze(Buffer buf, bool *hastup)
*hastup = false;
- /* If we hit an uninitialized page, we want to force vacuuming it. */
- if (PageIsNew(page))
- return true;
-
- /* Quick out for ordinary empty page. */
- if (PageIsEmpty(page))
+ /*
+ * New and empty pages, obviously, don't contain tuples. We could make
+ * sure that the page is registered in the FSM, but it doesn't seem worth
+ * waiting for a cleanup lock just for that, especially because it's
+ * likely that the pin holder will do so.
+ */
+ if (PageIsNew(page) || PageIsEmpty(page))
return false;
maxoff = PageGetMaxOffsetNumber(page);
@@ -2029,7 +2036,6 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
if (PageIsNew(page) || PageIsEmpty(page))
{
- /* PageIsNew probably shouldn't happen... */
UnlockReleaseBuffer(buf);
continue;
}
--
2.18.0.rc2.dirty
--sj3xhrbyfvv7pkyu--
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-02-02 07:34 Teodor Sigaev <[email protected]>
0 siblings, 2 replies; 39+ messages in thread
From: Teodor Sigaev @ 2022-02-02 07:34 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
> I agree ... but I'm also worried about what happens when we have
> multiple table AMs. One can imagine a new table AM that is
> specifically optimized for TOAST which can be used with an existing
> heap table. One can imagine a new table AM for the main table that
> wants to use something different for TOAST. So, I don't think it's
> right to imagine that the choice of TOASTer depends solely on the
> column data type. I'm not really sure how this should work exactly ...
> but it needs careful thought.
Right. that's why we propose a validate method (may be, it's a wrong
name, but I don't known better one) which accepts several arguments, one
of which is table AM oid. If that method returns false then toaster
isn't useful with current TAM, storage or/and compression kinds, etc.
--
Teodor Sigaev E-mail: [email protected]
WWW: http://www.sigaev.ru/
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-03-10 08:47 Nikita Malakhov <[email protected]>
parent: Teodor Sigaev <[email protected]>
1 sibling, 0 replies; 39+ messages in thread
From: Nikita Malakhov @ 2022-03-10 08:47 UTC (permalink / raw)
To: Teodor Sigaev <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
Hi Hackers,
In addition to original patch set for Pluggable Toaster, we have two more
patches
(actually, small, but important fixes), authored by Nikita Glukhov:
1) 0001-Fix-toast_tuple_externalize.patch
This patch fixes freeing memory in case of new toasted value is the same as
old one,
this seems incorrect, and in this case the function just returns instead of
freeing old value;
2) 0002-Fix-alignment-of-custom-TOAST-pointers.patch
This patch adds data alignment for new varatt_custom data structure in
building tuples,
since varatt_custom must be aligned for custom toasters (in particular,
this fix is very
important to JSONb Toaster).
These patches must be applied on top of the latter
4_bytea_appendable_toaster_v1.patch.
Please consider them in reviewing Pluggable Toaster.
Regards.
On Wed, Feb 2, 2022 at 10:35 AM Teodor Sigaev <[email protected]> wrote:
> > I agree ... but I'm also worried about what happens when we have
> > multiple table AMs. One can imagine a new table AM that is
> > specifically optimized for TOAST which can be used with an existing
> > heap table. One can imagine a new table AM for the main table that
> > wants to use something different for TOAST. So, I don't think it's
> > right to imagine that the choice of TOASTer depends solely on the
> > column data type. I'm not really sure how this should work exactly ...
> > but it needs careful thought.
>
> Right. that's why we propose a validate method (may be, it's a wrong
> name, but I don't known better one) which accepts several arguments, one
> of which is table AM oid. If that method returns false then toaster
> isn't useful with current TAM, storage or/and compression kinds, etc.
>
> --
> Teodor Sigaev E-mail: [email protected]
> WWW: http://www.sigaev.ru/
>
>
>
--
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/octet-stream] 0001-Fix-toast_tuple_externalize.patch (851B, ../../CAN-LCVORMn85jAL4GCWpQf+H+eZ3vFoO2r+tOEn7k47uw2A9gg@mail.gmail.com/3-0001-Fix-toast_tuple_externalize.patch)
download | inline diff:
From c16114bf17cee5b5649d3dfa462c4a8b594fb2f0 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <[email protected]>
Date: Tue, 22 Feb 2022 20:52:45 +0300
Subject: [PATCH 1/2] Fix toast_tuple_externalize()
---
src/backend/access/table/toast_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/backend/access/table/toast_helper.c b/src/backend/access/table/toast_helper.c
index bfbe941d6ac..11d870535e1 100644
--- a/src/backend/access/table/toast_helper.c
+++ b/src/backend/access/table/toast_helper.c
@@ -345,6 +345,9 @@ toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int maxDataLen,
maxDataLen, options)
);
+ if (*value == old_value)
+ return;
+
if ((attr->tai_colflags & TOASTCOL_NEEDS_FREE) != 0)
pfree(DatumGetPointer(old_value));
attr->tai_colflags |= TOASTCOL_NEEDS_FREE;
--
2.25.1
[application/octet-stream] 0002-Fix-alignment-of-custom-TOAST-pointers.patch (1.5K, ../../CAN-LCVORMn85jAL4GCWpQf+H+eZ3vFoO2r+tOEn7k47uw2A9gg@mail.gmail.com/4-0002-Fix-alignment-of-custom-TOAST-pointers.patch)
download | inline diff:
From 6db6630943be39eee31cb0411e2a849ec97bc66c Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <[email protected]>
Date: Wed, 12 Jan 2022 01:23:18 +0300
Subject: [PATCH 2/2] Fix alignment of custom TOAST pointers
---
src/backend/access/common/heaptuple.c | 5 ++++-
src/include/access/tupmacs.h | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 0b56b0fa5a9..47c808d462f 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -242,7 +242,10 @@ fill_val(Form_pg_attribute att,
else
{
*infomask |= HEAP_HASEXTERNAL;
- /* no alignment, since it's short by definition */
+ if (VARATT_IS_CUSTOM(val))
+ data = (char *) att_align_nominal(data,
+ att->attalign);
+ /* else no alignment, since it's short by definition */
data_length = VARSIZE_EXTERNAL(val);
memcpy(data, val, data_length);
}
diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h
index 65ac1ef3fc8..ddb164ef2ba 100644
--- a/src/include/access/tupmacs.h
+++ b/src/include/access/tupmacs.h
@@ -104,7 +104,7 @@
*/
#define att_align_datum(cur_offset, attalign, attlen, attdatum) \
( \
- ((attlen) == -1 && VARATT_IS_SHORT(DatumGetPointer(attdatum))) ? \
+ ((attlen) == -1 && (!VARATT_IS_CUSTOM(DatumGetPointer(attdatum)) && VARATT_IS_SHORT(DatumGetPointer(attdatum)))) ? \
(uintptr_t) (cur_offset) : \
att_align_nominal(cur_offset, attalign) \
)
--
2.25.1
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-03-21 23:31 Nikita Malakhov <[email protected]>
parent: Teodor Sigaev <[email protected]>
1 sibling, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-03-21 23:31 UTC (permalink / raw)
To: Teodor Sigaev <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
Hi Hackers,
Because of 3 months have passed since Pluggable Toaster presentation and a
lot of
commits were pushed into v15 master - we would like to re-introduce
this patch
rebased onto actual master. Last commit being used -
commit 641f3dffcdf1c7378cfb94c98b6642793181d6db (origin/master)
Author: Tom Lane <[email protected]>
Date: Fri Mar 11 13:47:26 2022 -0500
Updated patch consists of 4 patch files, next version (v2) of original
patch files
(please check original commit message from 30 Dec 2020):
1) 1_toaster_interface_v2.patch.gz
https://github.com/postgrespro/postgres/tree/toaster_interface
Introduces syntax for storage and formal toaster API.
2) 2_toaster_default_v2.patch.gz
https://github.com/postgrespro/postgres/tree/toaster_default
Built-in toaster implemented (with some refactoring) uisng toaster API
as generic (or default) toaster.
3) 3_toaster_snapshot_v2.patch.gz
https://github.com/postgrespro/postgres/tree/toaster_snapshot
The patch implements technology to distinguish row's versions in toasted
values to share common parts of toasted values between different
versions of rows
4) 4_bytea_appendable_toaster_v2.patch.gz
https://github.com/postgrespro/postgres/tree/bytea_appendable_toaster
Contrib module implements toaster for non-compressed bytea columns,
which allows fast appending to existing bytea value.
These patches also include 2 minor fixes made after commit fest presentation
1) Fix for freeing memory in case of new toasted value is the same as old
one,
this seems incorrect, and in this case the function just returns instead of
freeing old value;
2) Fix of data alignment for new varatt_custom data structure in building
tuples,
since varatt_custom must be aligned for custom toasters (in particular,
this fix is very
important to JSONb Toaster).
Thanks!
On Wed, Feb 2, 2022 at 10:35 AM Teodor Sigaev <[email protected]> wrote:
> > I agree ... but I'm also worried about what happens when we have
> > multiple table AMs. One can imagine a new table AM that is
> > specifically optimized for TOAST which can be used with an existing
> > heap table. One can imagine a new table AM for the main table that
> > wants to use something different for TOAST. So, I don't think it's
> > right to imagine that the choice of TOASTer depends solely on the
> > column data type. I'm not really sure how this should work exactly ...
> > but it needs careful thought.
>
> Right. that's why we propose a validate method (may be, it's a wrong
> name, but I don't known better one) which accepts several arguments, one
> of which is table AM oid. If that method returns false then toaster
> isn't useful with current TAM, storage or/and compression kinds, etc.
>
> --
> Teodor Sigaev E-mail: [email protected]
> WWW: http://www.sigaev.ru/
>
>
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 1_toaster_interface_v2.patch.gz (70.0K, ../../CAN-LCVNj0iLHRA4CzZHk3-BEt2-47Xbx1h020ReB0LA-=5H9YA@mail.gmail.com/3-1_toaster_interface_v2.patch.gz)
download
[application/x-gzip] 4_bytea_appendable_toaster_v2.patch.gz (154.5K, ../../CAN-LCVNj0iLHRA4CzZHk3-BEt2-47Xbx1h020ReB0LA-=5H9YA@mail.gmail.com/4-4_bytea_appendable_toaster_v2.patch.gz)
download
[application/x-gzip] 2_toaster_default_v2.patch.gz (130.4K, ../../CAN-LCVNj0iLHRA4CzZHk3-BEt2-47Xbx1h020ReB0LA-=5H9YA@mail.gmail.com/5-2_toaster_default_v2.patch.gz)
download
[application/x-gzip] 3_toaster_snapshot_v2.patch.gz (136.8K, ../../CAN-LCVNj0iLHRA4CzZHk3-BEt2-47Xbx1h020ReB0LA-=5H9YA@mail.gmail.com/6-3_toaster_snapshot_v2.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-03-22 00:51 Andres Freund <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Andres Freund @ 2022-03-22 00:51 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
On 2022-03-22 02:31:21 +0300, Nikita Malakhov wrote:
> Hi Hackers,
> Because of 3 months have passed since Pluggable Toaster presentation and a
> lot of
> commits were pushed into v15 master - we would like to re-introduce
> this patch
> rebased onto actual master. Last commit being used -
> commit 641f3dffcdf1c7378cfb94c98b6642793181d6db (origin/master)
> Author: Tom Lane <[email protected]>
> Date: Fri Mar 11 13:47:26 2022 -0500
It currently fails to apply: http://cfbot.cputube.org/patch_37_3490.log
Given the size of the patch, and the degree of review it has gotten so far, it
seems not realistically a fit for 15, but is marked as such.
Think it should be moved to the next CF. Marked as waiting-on-author for now.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-03-22 12:18 Nikita Malakhov <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-03-22 12:18 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
In the previous email I attached patches that were not sequential, all 4
files contained complete independent
patch to apply to master. Sorry, I re-created patch files to apply them in
sequence as in was meant in original
mail by Teodor Sigaev.
Please check.
Thanks!
On Tue, Mar 22, 2022 at 3:51 AM Andres Freund <[email protected]> wrote:
> Hi,
>
> On 2022-03-22 02:31:21 +0300, Nikita Malakhov wrote:
> > Hi Hackers,
> > Because of 3 months have passed since Pluggable Toaster presentation and
> a
> > lot of
> > commits were pushed into v15 master - we would like to re-introduce
> > this patch
> > rebased onto actual master. Last commit being used -
> > commit 641f3dffcdf1c7378cfb94c98b6642793181d6db (origin/master)
> > Author: Tom Lane <[email protected]>
> > Date: Fri Mar 11 13:47:26 2022 -0500
>
> It currently fails to apply: http://cfbot.cputube.org/patch_37_3490.log
>
> Given the size of the patch, and the degree of review it has gotten so
> far, it
> seems not realistically a fit for 15, but is marked as such.
>
> Think it should be moved to the next CF. Marked as waiting-on-author for
> now.
>
> Greetings,
>
> Andres Freund
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 4_bytea_appendable_toaster.patch.gz (19.5K, ../../CAN-LCVMvMvnopi3Q15o1sJRH8Tg+_T8USA0WKgWc-Nk330UkZQ@mail.gmail.com/3-4_bytea_appendable_toaster.patch.gz)
download
[application/x-gzip] 2_toaster_default.patch.gz (27.2K, ../../CAN-LCVMvMvnopi3Q15o1sJRH8Tg+_T8USA0WKgWc-Nk330UkZQ@mail.gmail.com/4-2_toaster_default.patch.gz)
download
[application/x-gzip] 1_toaster_interface.patch.gz (69.9K, ../../CAN-LCVMvMvnopi3Q15o1sJRH8Tg+_T8USA0WKgWc-Nk330UkZQ@mail.gmail.com/5-1_toaster_interface.patch.gz)
download
[application/x-gzip] 3_toaster_snapshot.patch.gz (7.4K, ../../CAN-LCVMvMvnopi3Q15o1sJRH8Tg+_T8USA0WKgWc-Nk330UkZQ@mail.gmail.com/6-3_toaster_snapshot.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-03-31 20:14 Greg Stark <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Greg Stark @ 2022-03-31 20:14 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Andres Freund <[email protected]>; Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
It looks like it's still not actually building on some compilers. I
see a bunch of warnings as well as an error:
[03:53:24.660] dummy_toaster.c:97:2: error: void function
'dummyDelete' should not return a value [-Wreturn-type]
Also the "publication" regression test needs to be adjusted as it
includes \d+ output which has changed to include the toaster.
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-01 14:11 Nikita Malakhov <[email protected]>
parent: Greg Stark <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-01 14:11 UTC (permalink / raw)
To: Greg Stark <[email protected]>; +Cc: Andres Freund <[email protected]>; Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
Have found code corrupted by merge went unnoticed in dummy_toaster.c.
Please look at the attached patches. They must be applied sequentially, one
after another,
from the 1st one. I haven't seed any warnings during compilation (with gcc
on Ubuntu 20),
and check-world goes with no errors.
On Thu, Mar 31, 2022 at 11:15 PM Greg Stark <[email protected]> wrote:
> It looks like it's still not actually building on some compilers. I
> see a bunch of warnings as well as an error:
>
> [03:53:24.660] dummy_toaster.c:97:2: error: void function
> 'dummyDelete' should not return a value [-Wreturn-type]
>
> Also the "publication" regression test needs to be adjusted as it
> includes \d+ output which has changed to include the toaster.
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 1_toaster_interface_v3.patch.gz (70.0K, ../../CAN-LCVOA9y8enbxfci9O28TeETARBrrnuu0aBowS=MSBMJnWaA@mail.gmail.com/3-1_toaster_interface_v3.patch.gz)
download
[application/x-gzip] 2_toaster_default_v3.patch.gz (27.3K, ../../CAN-LCVOA9y8enbxfci9O28TeETARBrrnuu0aBowS=MSBMJnWaA@mail.gmail.com/4-2_toaster_default_v3.patch.gz)
download
[application/x-gzip] 3_toaster_snapshot_v3.patch.gz (7.8K, ../../CAN-LCVOA9y8enbxfci9O28TeETARBrrnuu0aBowS=MSBMJnWaA@mail.gmail.com/5-3_toaster_snapshot_v3.patch.gz)
download
[application/x-gzip] 4_bytea_appendable_toaster_v3.patch.gz (19.5K, ../../CAN-LCVOA9y8enbxfci9O28TeETARBrrnuu0aBowS=MSBMJnWaA@mail.gmail.com/6-4_bytea_appendable_toaster_v3.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-03 01:20 Greg Stark <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Greg Stark @ 2022-04-03 01:20 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Andres Freund <[email protected]>; Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hm. It compiles but it's failing regression tests:
diff -U3 /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
/tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
--- /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
2022-04-02 16:02:47.874360253 +0000
+++ /tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
2022-04-02 16:07:20.878047769 +0000
@@ -20,186 +20,7 @@
....
+server closed the connection unexpectedly
+ This probably means the server terminated abnormally
+ before or while processing the request.
+connection to server was lost
I think this will require some real debugging, so I'm marking this
Waiting on Author.
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-03 02:06 Andres Freund <[email protected]>
parent: Greg Stark <[email protected]>
0 siblings, 2 replies; 39+ messages in thread
From: Andres Freund @ 2022-04-03 02:06 UTC (permalink / raw)
To: Greg Stark <[email protected]>; Nikita Malakhov <[email protected]>; +Cc: Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
On April 2, 2022 6:20:36 PM PDT, Greg Stark <[email protected]> wrote:
>Hm. It compiles but it's failing regression tests:
>
>diff -U3 /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
>/tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
>--- /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
>2022-04-02 16:02:47.874360253 +0000
>+++ /tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
>2022-04-02 16:07:20.878047769 +0000
>@@ -20,186 +20,7 @@
>....
>+server closed the connection unexpectedly
>+ This probably means the server terminated abnormally
>+ before or while processing the request.
>+connection to server was lost
>I think this will require some real debugging, so I'm marking this
>Waiting on Author.
Yes, dumps core (just like in several previous runs):
https://cirrus-ci.com/task/4710272324599808?logs=cores#L44
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-03 13:15 Nikita Malakhov <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-03 13:15 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
I'm checking. It seems that I've missed something while rebasing, we have
had all tests clean before.
On Sun, Apr 3, 2022 at 5:06 AM Andres Freund <[email protected]> wrote:
> Hi,
>
> On April 2, 2022 6:20:36 PM PDT, Greg Stark <[email protected]> wrote:
> >Hm. It compiles but it's failing regression tests:
> >
> >diff -U3
> /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
> >/tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
> >--- /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
> >2022-04-02 16:02:47.874360253 +0000
> >+++ /tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
> >2022-04-02 16:07:20.878047769 +0000
> >@@ -20,186 +20,7 @@
> >....
> >+server closed the connection unexpectedly
> >+ This probably means the server terminated abnormally
> >+ before or while processing the request.
> >+connection to server was lost
> >I think this will require some real debugging, so I'm marking this
> >Waiting on Author.
>
> Yes, dumps core (just like in several previous runs):
>
> https://cirrus-ci.com/task/4710272324599808?logs=cores#L44
>
> Andres
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-04 16:12 Nikita Malakhov <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-04 16:12 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
I'm really sorry. Messed up some code while merging rebased branches with
previous (v1)
patches issued in December and haven't noticed that seg fault because of
corrupted code
while running check-world.
I've fixed messed code in Dummy Toaster package and checked twice - all's
correct now,
patches are applied correctly and tests are clean.
Thank you for pointing out the error and for your patience!
On Sun, Apr 3, 2022 at 5:06 AM Andres Freund <[email protected]> wrote:
> Hi,
>
> On April 2, 2022 6:20:36 PM PDT, Greg Stark <[email protected]> wrote:
> >Hm. It compiles but it's failing regression tests:
> >
> >diff -U3
> /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
> >/tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
> >--- /tmp/cirrus-ci-build/contrib/dummy_toaster/expected/dummy_toaster.out
> >2022-04-02 16:02:47.874360253 +0000
> >+++ /tmp/cirrus-ci-build/contrib/dummy_toaster/results/dummy_toaster.out
> >2022-04-02 16:07:20.878047769 +0000
> >@@ -20,186 +20,7 @@
> >....
> >+server closed the connection unexpectedly
> >+ This probably means the server terminated abnormally
> >+ before or while processing the request.
> >+connection to server was lost
> >I think this will require some real debugging, so I'm marking this
> >Waiting on Author.
>
> Yes, dumps core (just like in several previous runs):
>
> https://cirrus-ci.com/task/4710272324599808?logs=cores#L44
>
> Andres
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 3_toaster_snapshot_v4.patch.gz (7.7K, ../../CAN-LCVN8WMk0zbJrqpq3wQFPvunMjsdCj4y5gyEuyH0+4Ax-DA@mail.gmail.com/3-3_toaster_snapshot_v4.patch.gz)
download
[application/x-gzip] 4_bytea_appendable_toaster_v4.patch.gz (19.5K, ../../CAN-LCVN8WMk0zbJrqpq3wQFPvunMjsdCj4y5gyEuyH0+4Ax-DA@mail.gmail.com/4-4_bytea_appendable_toaster_v4.patch.gz)
download
[application/x-gzip] 2_toaster_default_v4.patch.gz (27.4K, ../../CAN-LCVN8WMk0zbJrqpq3wQFPvunMjsdCj4y5gyEuyH0+4Ax-DA@mail.gmail.com/5-2_toaster_default_v4.patch.gz)
download
[application/x-gzip] 1_toaster_interface_v4.patch.gz (70.2K, ../../CAN-LCVN8WMk0zbJrqpq3wQFPvunMjsdCj4y5gyEuyH0+4Ax-DA@mail.gmail.com/6-1_toaster_interface_v4.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-04 16:39 Robert Haas <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 2 replies; 39+ messages in thread
From: Robert Haas @ 2022-04-04 16:39 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
On Mon, Apr 4, 2022 at 12:13 PM Nikita Malakhov <[email protected]> wrote:
> I'm really sorry. Messed up some code while merging rebased branches with previous (v1)
> patches issued in December and haven't noticed that seg fault because of corrupted code
> while running check-world.
> I've fixed messed code in Dummy Toaster package and checked twice - all's correct now,
> patches are applied correctly and tests are clean.
> Thank you for pointing out the error and for your patience!
Hi,
This patch set doesn't seem anywhere close to committable to me. For example:
- It apparently adds a new command called CREATE TOASTER but that
command doesn't seem to be documented anywhere.
- contrib/dummy_toaster is added in patch 1 with a no implementation
and code comments that say "Bloom index utilities" and then those
comments are fixed and an implementation is added in later patches.
- What is supposedly patch 1 is actually 32 patch files concatenated
together. It doesn't apply properly either with 'patch' or 'git am' so
I don't understand what we would even do with this.
- None of these patches have appropriate commit messages.
- Many of these patches add 'XXX' comments or errors or other
obviously unfinished bits of code. Some of this may be cleaned up by
later patches but it's hard to tell because right now one can't even
apply the patch set properly. Even if that were possible, it's the job
of the person submitting the patch to organize the patch into
independent, separately committable chunks that are self-contained and
have good comments and good commit messages for each one.
I don't think based on the status of this patch set that it's even
possible to provide useful feedback on the design at this point, never
mind getting anything committed.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-04 19:32 Greg Stark <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 0 replies; 39+ messages in thread
From: Greg Stark @ 2022-04-04 19:32 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Nikita Malakhov <[email protected]>; Andres Freund <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
Thanks for the review Robert. I think that gives some pretty
actionable advice on how to improve the patch and it doesn't seem
likely to get much more in this cycle.
I'll mark the patch Returned with Feedback. Hope to see it come back
with improvements in the next release.
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-04 20:05 Nikita Malakhov <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-04 20:05 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
Hi,
Thank you very much for your review, I'd like to get it much earlier. I'm
currently
working on cleaning up code, and would try to comment as much as possible.
This patch set is really a large set of functionality, it was divided into
4 logically complete
parts, but anyway these parts contain a lot of changes themselves.
- Yes, you're right, new syntax is added. I'm also working on the
documentation part for it;
- Patches actually consist of a lot of minor commits. As I see we have to
squash them
to provide parts as 2-3 main commits without any unnecessary garbage;
- Is 'git apply' not a valid way to apply such patches?
We'll try to straighten these issues out asap.
Thank you!
On Mon, Apr 4, 2022 at 7:40 PM Robert Haas <[email protected]> wrote:
> On Mon, Apr 4, 2022 at 12:13 PM Nikita Malakhov <[email protected]> wrote:
> > I'm really sorry. Messed up some code while merging rebased branches
> with previous (v1)
> > patches issued in December and haven't noticed that seg fault because of
> corrupted code
> > while running check-world.
> > I've fixed messed code in Dummy Toaster package and checked twice -
> all's correct now,
> > patches are applied correctly and tests are clean.
> > Thank you for pointing out the error and for your patience!
>
> Hi,
>
> This patch set doesn't seem anywhere close to committable to me. For
> example:
>
> - It apparently adds a new command called CREATE TOASTER but that
> command doesn't seem to be documented anywhere.
>
> - contrib/dummy_toaster is added in patch 1 with a no implementation
> and code comments that say "Bloom index utilities" and then those
> comments are fixed and an implementation is added in later patches.
>
> - What is supposedly patch 1 is actually 32 patch files concatenated
> together. It doesn't apply properly either with 'patch' or 'git am' so
> I don't understand what we would even do with this.
>
> - None of these patches have appropriate commit messages.
>
> - Many of these patches add 'XXX' comments or errors or other
> obviously unfinished bits of code. Some of this may be cleaned up by
> later patches but it's hard to tell because right now one can't even
> apply the patch set properly. Even if that were possible, it's the job
> of the person submitting the patch to organize the patch into
> independent, separately committable chunks that are self-contained and
> have good comments and good commit messages for each one.
>
> I don't think based on the status of this patch set that it's even
> possible to provide useful feedback on the design at this point, never
> mind getting anything committed.
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-04 20:17 Robert Haas <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 2 replies; 39+ messages in thread
From: Robert Haas @ 2022-04-04 20:17 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
On Mon, Apr 4, 2022 at 4:05 PM Nikita Malakhov <[email protected]> wrote:
> - Is 'git apply' not a valid way to apply such patches?
I have found that it never works. This case is no exception:
[rhaas pgsql]$ git apply ~/Downloads/1_toaster_interface_v4.patch
/Users/rhaas/Downloads/1_toaster_interface_v4.patch:253: trailing whitespace.
toasterapi.o
/Users/rhaas/Downloads/1_toaster_interface_v4.patch:1276: trailing whitespace.
{
/Users/rhaas/Downloads/1_toaster_interface_v4.patch:1294: trailing whitespace.
* CREATE TOASTER name HANDLER handler_name
/Users/rhaas/Downloads/1_toaster_interface_v4.patch:2261: trailing whitespace.
* va_toasterdata could contain varatt_external structure for old Toast
/Users/rhaas/Downloads/1_toaster_interface_v4.patch:3047: trailing whitespace.
SELECT attnum, attname, atttypid, attstorage, tsrname
error: patch failed: src/backend/commands/tablecmds.c:42
error: src/backend/commands/tablecmds.c: patch does not apply
error: patch failed: src/backend/commands/tablecmds.c:943
error: src/backend/commands/tablecmds.c: patch does not apply
error: patch failed: src/backend/commands/tablecmds.c:973
error: src/backend/commands/tablecmds.c: patch does not apply
error: patch failed: src/backend/commands/tablecmds.c:44
error: src/backend/commands/tablecmds.c: patch does not apply
I would really encourage you to use 'git format-patch' to generate a
stack of patches. But there is no point in reposting 30+ patches that
haven't been properly refactored into separate chunks. You need to
maintain a branch, periodically rebased over master, with some
probably-small number of patches on it, each of which is a logically
independent patch with its own commit message, its own clear purpose,
etc. And then generate patches to post from there using 'git
format-patch'. Look into using 'git rebase -i --autosquash' and 'git
commit --fixup' to maintain the branch, if you're not already familiar
with those things.
Also, it is a really good idea when you post the patch set to include
in the email a clear description of the overall purpose of the patch
set and what each patch does toward that goal. e.g. "The overall goal
of this patch set is to support faster-than-light travel. Currently,
PostgreSQL does not know anything about the speed of light, so 0001
adds some code for speed-of-light detection. Building on this, 0002
adds general support for disabling physical laws of the universe.
Then, 0003 makes use of this support to disable specifically the speed
of light." Perhaps you want a little more text than that for each
patch, depending on the situation, but this gives you the idea, I
hope.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-05 07:58 Nikita Malakhov <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 0 replies; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-05 07:58 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
Hi,
Thanks for advices.
We have 4 branches, for each patch provided, you can check them out -
(come copy-paste from the very fist email, where the patches were proposed)
1) 1_toaster_interface
https://github.com/postgrespro/postgres/tree/toaster_interface
Introduces syntax for storage and formal toaster API. Adds column
atttoaster to pg_attribute, by design this column should not be equal to
invalid oid for any toastable datatype, ie it must have correct oid for
any type (not column) with non-plain storage. Since toaster may support
only particular datatype, core should check correctness of toaster set
by toaster validate method. New commands could be found in
src/test/regress/sql/toaster.sql. Also includes modification of pg_dump.
2) 2_toaster_default
https://github.com/postgrespro/postgres/tree/toaster_default
Built-in toaster implemented (with some refactoring) using toaster API
as generic (or default) toaster. dummy_toaster here is a minimal
workable example, it saves value directly in toast pointer and fails if
value is greater than 1kb.
3) 3_toaster_snapshot
https://github.com/postgrespro/postgres/tree/toaster_snapshot
The patch implements technology to distinguish row's versions in toasted
values to share common parts of toasted values between different
versions of rows
4) 4_bytea_appendable_toaster
https://github.com/postgrespro/postgres/tree/bytea_appendable_toaster
Contrib module implements toaster for non-compressed bytea columns,
which allows fast appending to existing bytea value. Appended tail
stored directly in toaster pointer, if there is enough space to do it.
Working on refactoring according to your recommendations.
Thank you!
On Mon, Apr 4, 2022 at 11:18 PM Robert Haas <[email protected]> wrote:
> On Mon, Apr 4, 2022 at 4:05 PM Nikita Malakhov <[email protected]> wrote:
> > - Is 'git apply' not a valid way to apply such patches?
>
> I have found that it never works. This case is no exception:
>
> [rhaas pgsql]$ git apply ~/Downloads/1_toaster_interface_v4.patch
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:253: trailing
> whitespace.
> toasterapi.o
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1276: trailing
> whitespace.
> {
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1294: trailing
> whitespace.
> * CREATE TOASTER name HANDLER handler_name
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:2261: trailing
> whitespace.
> * va_toasterdata could contain varatt_external structure for old Toast
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:3047: trailing
> whitespace.
> SELECT attnum, attname, atttypid, attstorage, tsrname
> error: patch failed: src/backend/commands/tablecmds.c:42
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:943
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:973
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:44
> error: src/backend/commands/tablecmds.c: patch does not apply
>
> I would really encourage you to use 'git format-patch' to generate a
> stack of patches. But there is no point in reposting 30+ patches that
> haven't been properly refactored into separate chunks. You need to
> maintain a branch, periodically rebased over master, with some
> probably-small number of patches on it, each of which is a logically
> independent patch with its own commit message, its own clear purpose,
> etc. And then generate patches to post from there using 'git
> format-patch'. Look into using 'git rebase -i --autosquash' and 'git
> commit --fixup' to maintain the branch, if you're not already familiar
> with those things.
>
> Also, it is a really good idea when you post the patch set to include
> in the email a clear description of the overall purpose of the patch
> set and what each patch does toward that goal. e.g. "The overall goal
> of this patch set is to support faster-than-light travel. Currently,
> PostgreSQL does not know anything about the speed of light, so 0001
> adds some code for speed-of-light detection. Building on this, 0002
> adds general support for disabling physical laws of the universe.
> Then, 0003 makes use of this support to disable specifically the speed
> of light." Perhaps you want a little more text than that for each
> patch, depending on the situation, but this gives you the idea, I
> hope.
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-13 18:55 Nikita Malakhov <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-13 18:55 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
Hi,
I reworked previous patch set according to recommendations. Patches
are generated by format-patch and applied by git am. Patches are based on
master from 03.11. Also, now we've got clean branch with incremental
commits
which could be easily rebased onto a fresh master.
Currently, there are 8 patches:
1) 0001_create_table_storage_v3.patch - SET STORAGE option for CREATE
TABLE command by Teodor Sigaev which is required by all the following
functionality;
2) 0002_toaster_interface_v6.patch - Toaster API (SQL syntax for toasters +
API)
with Dummy toaster as an example of how this API should be used, but with
default
toaster left 'as-is';
3) 0003_toaster_default_v5.patch - default (regular) toaster is implemented
via new API;
4) 0004_toaster_snapshot_v5.patch - refactoring of default toaster and
support
of versioned toasted rows;
5) 0005_bytea_appendable_toaster_v5.patch - bytea toaster by Nikita Glukhov
Custom toaster for bytea data with support of appending (instead of
rewriting)
stored data;
6) 0006_toasterapi_docs_v1.patch - brief documentation on Toaster API in Pg
docs;
7) 0007_fix_alignment_of_custom_toast_pointers.patch - fixes custom toast
pointer's
alignment required by bytea toaster by Nikita Glukhov;
8) 0008_fix_toast_tuple_externalize.patch - fixes toast_tuple_externalize
function
not to call toast if old data is the same as new one.
I would be grateful for feedback on the reworked patch set.
On Mon, Apr 4, 2022 at 11:18 PM Robert Haas <[email protected]> wrote:
> On Mon, Apr 4, 2022 at 4:05 PM Nikita Malakhov <[email protected]> wrote:
> > - Is 'git apply' not a valid way to apply such patches?
>
> I have found that it never works. This case is no exception:
>
> [rhaas pgsql]$ git apply ~/Downloads/1_toaster_interface_v4.patch
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:253: trailing
> whitespace.
> toasterapi.o
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1276: trailing
> whitespace.
> {
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1294: trailing
> whitespace.
> * CREATE TOASTER name HANDLER handler_name
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:2261: trailing
> whitespace.
> * va_toasterdata could contain varatt_external structure for old Toast
> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:3047: trailing
> whitespace.
> SELECT attnum, attname, atttypid, attstorage, tsrname
> error: patch failed: src/backend/commands/tablecmds.c:42
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:943
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:973
> error: src/backend/commands/tablecmds.c: patch does not apply
> error: patch failed: src/backend/commands/tablecmds.c:44
> error: src/backend/commands/tablecmds.c: patch does not apply
>
> I would really encourage you to use 'git format-patch' to generate a
> stack of patches. But there is no point in reposting 30+ patches that
> haven't been properly refactored into separate chunks. You need to
> maintain a branch, periodically rebased over master, with some
> probably-small number of patches on it, each of which is a logically
> independent patch with its own commit message, its own clear purpose,
> etc. And then generate patches to post from there using 'git
> format-patch'. Look into using 'git rebase -i --autosquash' and 'git
> commit --fixup' to maintain the branch, if you're not already familiar
> with those things.
>
> Also, it is a really good idea when you post the patch set to include
> in the email a clear description of the overall purpose of the patch
> set and what each patch does toward that goal. e.g. "The overall goal
> of this patch set is to support faster-than-light travel. Currently,
> PostgreSQL does not know anything about the speed of light, so 0001
> adds some code for speed-of-light detection. Building on this, 0002
> adds general support for disabling physical laws of the universe.
> Then, 0003 makes use of this support to disable specifically the speed
> of light." Perhaps you want a little more text than that for each
> patch, depending on the situation, but this gives you the idea, I
> hope.
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 0005_bytea_appendable_toaster_v5.patch.gz (5.6K, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/3-0005_bytea_appendable_toaster_v5.patch.gz)
download
[application/x-gzip] 0004_toaster_snapshot_v5.patch.gz (8.1K, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/4-0004_toaster_snapshot_v5.patch.gz)
download
[application/x-gzip] 0001_create_table_storage_v3.patch.gz (4.3K, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/5-0001_create_table_storage_v3.patch.gz)
download
[application/x-gzip] 0003_toaster_default_v5.patch.gz (30.0K, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/6-0003_toaster_default_v5.patch.gz)
download
[application/x-gzip] 0002_toaster_interface_v6.patch.gz (46.0K, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/7-0002_toaster_interface_v6.patch.gz)
download
[application/x-gzip] 0006_toasterapi_docs_v1.patch.gz (3.9K, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/8-0006_toasterapi_docs_v1.patch.gz)
download
[application/x-gzip] 0008_fix_toast_tuple_externalize.patch.gz (579B, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/9-0008_fix_toast_tuple_externalize.patch.gz)
download
[application/x-gzip] 0007_fix_alignment_of_custom_toast_pointers.patch.gz (780B, ../../CAN-LCVPximi0n6C7CN83Xq8-jpBw0cOaC9fkBHNr0qo7Z4Fsjw@mail.gmail.com/10-0007_fix_alignment_of_custom_toast_pointers.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-04-13 19:58 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-04-13 19:58 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; pgsql-hackers
Hi,
For a pluggable toaster - in previous patch set part 7 patch file contains
invalid string.
Fixup (v2 file should used instead of previous) patch:
7) 0007_fix_alignment_of_custom_toast_pointers.patch - fixes custom toast
pointer's
alignment required by bytea toaster by Nikita Glukhov;
On Wed, Apr 13, 2022 at 9:55 PM Nikita Malakhov <[email protected]> wrote:
> Hi,
> I reworked previous patch set according to recommendations. Patches
> are generated by format-patch and applied by git am. Patches are based on
> master from 03.11. Also, now we've got clean branch with incremental
> commits
> which could be easily rebased onto a fresh master.
>
> Currently, there are 8 patches:
>
> 1) 0001_create_table_storage_v3.patch - SET STORAGE option for CREATE
> TABLE command by Teodor Sigaev which is required by all the following
> functionality;
>
> 2) 0002_toaster_interface_v6.patch - Toaster API (SQL syntax for toasters
> + API)
> with Dummy toaster as an example of how this API should be used, but with
> default
> toaster left 'as-is';
>
> 3) 0003_toaster_default_v5.patch - default (regular) toaster is implemented
> via new API;
>
> 4) 0004_toaster_snapshot_v5.patch - refactoring of default toaster and
> support
> of versioned toasted rows;
>
> 5) 0005_bytea_appendable_toaster_v5.patch - bytea toaster by Nikita Glukhov
> Custom toaster for bytea data with support of appending (instead of
> rewriting)
> stored data;
>
> 6) 0006_toasterapi_docs_v1.patch - brief documentation on Toaster API in
> Pg docs;
>
> 7) 0007_fix_alignment_of_custom_toast_pointers.patch - fixes custom toast
> pointer's
> alignment required by bytea toaster by Nikita Glukhov;
>
> 8) 0008_fix_toast_tuple_externalize.patch - fixes toast_tuple_externalize
> function
> not to call toast if old data is the same as new one.
>
> I would be grateful for feedback on the reworked patch set.
>
> On Mon, Apr 4, 2022 at 11:18 PM Robert Haas <[email protected]> wrote:
>
>> On Mon, Apr 4, 2022 at 4:05 PM Nikita Malakhov <[email protected]> wrote:
>> > - Is 'git apply' not a valid way to apply such patches?
>>
>> I have found that it never works. This case is no exception:
>>
>> [rhaas pgsql]$ git apply ~/Downloads/1_toaster_interface_v4.patch
>> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:253: trailing
>> whitespace.
>> toasterapi.o
>> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1276: trailing
>> whitespace.
>> {
>> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:1294: trailing
>> whitespace.
>> * CREATE TOASTER name HANDLER handler_name
>> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:2261: trailing
>> whitespace.
>> * va_toasterdata could contain varatt_external structure for old Toast
>> /Users/rhaas/Downloads/1_toaster_interface_v4.patch:3047: trailing
>> whitespace.
>> SELECT attnum, attname, atttypid, attstorage, tsrname
>> error: patch failed: src/backend/commands/tablecmds.c:42
>> error: src/backend/commands/tablecmds.c: patch does not apply
>> error: patch failed: src/backend/commands/tablecmds.c:943
>> error: src/backend/commands/tablecmds.c: patch does not apply
>> error: patch failed: src/backend/commands/tablecmds.c:973
>> error: src/backend/commands/tablecmds.c: patch does not apply
>> error: patch failed: src/backend/commands/tablecmds.c:44
>> error: src/backend/commands/tablecmds.c: patch does not apply
>>
>> I would really encourage you to use 'git format-patch' to generate a
>> stack of patches. But there is no point in reposting 30+ patches that
>> haven't been properly refactored into separate chunks. You need to
>> maintain a branch, periodically rebased over master, with some
>> probably-small number of patches on it, each of which is a logically
>> independent patch with its own commit message, its own clear purpose,
>> etc. And then generate patches to post from there using 'git
>> format-patch'. Look into using 'git rebase -i --autosquash' and 'git
>> commit --fixup' to maintain the branch, if you're not already familiar
>> with those things.
>>
>> Also, it is a really good idea when you post the patch set to include
>> in the email a clear description of the overall purpose of the patch
>> set and what each patch does toward that goal. e.g. "The overall goal
>> of this patch set is to support faster-than-light travel. Currently,
>> PostgreSQL does not know anything about the speed of light, so 0001
>> adds some code for speed-of-light detection. Building on this, 0002
>> adds general support for disabling physical laws of the universe.
>> Then, 0003 makes use of this support to disable specifically the speed
>> of light." Perhaps you want a little more text than that for each
>> patch, depending on the situation, but this gives you the idea, I
>> hope.
>>
>> --
>> Robert Haas
>> EDB: http://www.enterprisedb.com
>>
>
>
> --
> Regards,
> Nikita Malakhov
> Postgres Professional
> https://postgrespro.ru/
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 0007_fix_alignment_of_custom_toast_pointers_v2.patch.gz (801B, ../../CAN-LCVO2L51gMCsM4-v=NFmDt1qh+HWL4GJXzfxaQZX6cwofkw@mail.gmail.com/3-0007_fix_alignment_of_custom_toast_pointers_v2.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-06-17 14:33 Aleksander Alekseev <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Aleksander Alekseev @ 2022-06-17 14:33 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Nikita Malakhov <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Matthias van de Meent <[email protected]>
Hi hackers,
> For a pluggable toaster - in previous patch set part 7 patch file contains invalid string.
> Fixup (v2 file should used instead of previous) patch:
> 7) 0007_fix_alignment_of_custom_toast_pointers.patch - fixes custom toast pointer's
> alignment required by bytea toaster by Nikita Glukhov;
I finished digesting the thread and the referred presentations per
Matthias (cc:'ed) suggestion in [1] discussion. Although the patchset
got a fair amount of push-back above, I prefer to stay open minded and
invest some of my time into this effort as a tester/reviewer during
the July CF. Even if the patchset will not make it entirely to the
core, some of its parts can be useful.
Unfortunately, I didn't manage to find something that can be applied
and tested. cfbot is currently not happy with the patchset.
0001_create_table_storage_v3.patch doesn't apply to the current
origin/master manually either:
```
error: patch failed: src/backend/parser/gram.y:2318
error: src/backend/parser/gram.y: patch does not apply
```
Any chance we can see a rebased patchset for the July CF?
[1]: https://commitfest.postgresql.org/38/3626/
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-06-23 11:46 Nikita Malakhov <[email protected]>
parent: Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-06-23 11:46 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Matthias van de Meent <[email protected]>
Hi hackers,
We're currently working on rebase along other TOAST improvements, hope to
do it in time for July CF.
Thank you for your patience.
--
Best regards,
Nikita Malakhov
On Fri, Jun 17, 2022 at 5:33 PM Aleksander Alekseev <
[email protected]> wrote:
> Hi hackers,
>
> > For a pluggable toaster - in previous patch set part 7 patch file
> contains invalid string.
> > Fixup (v2 file should used instead of previous) patch:
> > 7) 0007_fix_alignment_of_custom_toast_pointers.patch - fixes custom
> toast pointer's
> > alignment required by bytea toaster by Nikita Glukhov;
>
> I finished digesting the thread and the referred presentations per
> Matthias (cc:'ed) suggestion in [1] discussion. Although the patchset
> got a fair amount of push-back above, I prefer to stay open minded and
> invest some of my time into this effort as a tester/reviewer during
> the July CF. Even if the patchset will not make it entirely to the
> core, some of its parts can be useful.
>
> Unfortunately, I didn't manage to find something that can be applied
> and tested. cfbot is currently not happy with the patchset.
> 0001_create_table_storage_v3.patch doesn't apply to the current
> origin/master manually either:
>
> ```
> error: patch failed: src/backend/parser/gram.y:2318
> error: src/backend/parser/gram.y: patch does not apply
> ```
>
> Any chance we can see a rebased patchset for the July CF?
>
> [1]: https://commitfest.postgresql.org/38/3626/
>
> --
> Best regards,
> Aleksander Alekseev
>
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-06-23 13:38 Aleksander Alekseev <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Aleksander Alekseev @ 2022-06-23 13:38 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Robert Haas <[email protected]>; Nikita Malakhov <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Matthias van de Meent <[email protected]>
Hi Nikita,
> We're currently working on rebase along other TOAST improvements, hope to do it in time for July CF.
> Thank you for your patience.
Just to clarify, does it include the dependent "CREATE TABLE ( ..
STORAGE .. )" patch [1]? I was considering changing the patch
according to the feedback it got, but if you are already working on
this I'm not going to interfere.
[1]: https://postgr.es/m/de83407a-ae3d-a8e1-a788-920eb334f25b%40sigaev.ru
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-06-23 13:53 Nikita Malakhov <[email protected]>
parent: Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-06-23 13:53 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Matthias van de Meent <[email protected]>
Hi,
Alexander, thank you for your feedback and willingness to help. You can
send a suggested fixup in this thread, I'll check the issue
you've mentioned.
Best regards,
Nikita Malakhov
On Thu, Jun 23, 2022 at 4:38 PM Aleksander Alekseev <
[email protected]> wrote:
> Hi Nikita,
>
> > We're currently working on rebase along other TOAST improvements, hope
> to do it in time for July CF.
> > Thank you for your patience.
>
> Just to clarify, does it include the dependent "CREATE TABLE ( ..
> STORAGE .. )" patch [1]? I was considering changing the patch
> according to the feedback it got, but if you are already working on
> this I'm not going to interfere.
>
> [1]: https://postgr.es/m/de83407a-ae3d-a8e1-a788-920eb334f25b%40sigaev.ru
> --
> Best regards,
> Aleksander Alekseev
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-06-30 20:26 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 2 replies; 39+ messages in thread
From: Nikita Malakhov @ 2022-06-30 20:26 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>; Matthias van de Meent <[email protected]>
Hi hackers!
Here is the patch set rebased onto current master (15 rel beta 2 with
commit from 29.06).
Just to remind:
In Pluggable TOAST we suggest a way to make TOAST pluggable as Storage (in
a way like Pluggable Access Methods) - we extracted
TOAST mechanics from Heap AM, and made it an independent pluggable and
extensible part with our freshly developed TOAST API.
With this patch set you will be able to develop and plug in your own TOAST
mechanics for table columns. Knowing internals and/or workflow and workload
of data being TOASTed makes Custom Toasters much more efficient in
performance and storage.
We keep backwards compatibility and default TOAST mechanics works as it
worked previously, working silently with any Toastable datatype
(and TOASTed values and tables from previous versions, no changes in this)
and set as default Toaster is not stated otherwise, but through our TOAST
API.
TOAST API does not have any noticeable overhead in comparison to the
original (master). Proofs in our research materials (measured).
We've already presented out work at HighLoad, PgCon and PgConf conferences,
you can find materials here
http://www.sai.msu.su/~megera/postgres/talks/
We have ready to plug in extension Toasters
- bytea appendable toaster for bytea datatype (impressive speedup with
bytea append operation)
- JSONB toaster for JSONB (very cool performance improvements when dealing
with TOASTed JSONB)
and prototype Toasters (in development) for PostGIS (much faster then
default with geometric data), large binary objects
(like pg_largeobject, but much, much larger, and without existing large
object limitations), default Toaster implementation without using Indexes.
Patch set consists of 9 incremental patches:
0001_create_table_storage_v4.patch - SQL syntax fix for CREATE TABLE
clause, processing SET STORAGE... correctly;
0002_toaster_interface_v7.patch - TOAST API interface and SQL syntax
allowing creation of custom Toaster (CREATE TOASTER ...)
and setting Toaster to a table column (CREATE TABLE t (data bytea STORAGE
EXTERNAL TOASTER bytea_toaster);)
0003_toaster_default_v6.patch - Default TOAST implemented via TOAST API;
0004_toaster_snapshot_v6.patch - refactoring of Default TOAST and support
for versioned Toast rows;
0005_bytea_appendable_toaster_v6.patch - contrib module
bytea_appendable_toaster - special Toaster for bytea datatype with
customized append operation;
0006_toasterapi_docs_v2.patch - documentation package for Pluggable TOAST;
0007_fix_alignment_of_custom_toast_pointers_v2.patch - fixes custom toast
pointer's
alignment required by bytea toaster by Nikita Glukhov;
0008_fix_toast_tuple_externalize_v2.patch - fixes toast_tuple_externalize
function
not to call toast if old data is the same as new one.
0009_bytea_contrib_and_varlena_v1.patch - several late fixups for 0005.
This patch set opens the following issues:
1) With TOAST independent of AM it is used by it makes sense to move
compression from AM into Toaster and make Compression one of Toaster's
options.
Actually, Toasters allow to use any compression methods independently of AM;
2) Implement default Toaster without using Indexes (currently in
development)?
3) Allows different, SQL-accessed large objects of almost infinite size IN
DATABASE, unlike current large_object functionality and does not limit
their quantity;
4) Several already developed Toasters show impressive results for
datatypes they were designed for.
We're gladly appreciate your feedback!
--
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
On Thu, Jun 23, 2022 at 4:53 PM Nikita Malakhov <[email protected]> wrote:
> Hi,
> Alexander, thank you for your feedback and willingness to help. You can
> send a suggested fixup in this thread, I'll check the issue
> you've mentioned.
>
> Best regards,
> Nikita Malakhov
>
> On Thu, Jun 23, 2022 at 4:38 PM Aleksander Alekseev <
> [email protected]> wrote:
>
>> Hi Nikita,
>>
>> > We're currently working on rebase along other TOAST improvements, hope
>> to do it in time for July CF.
>> > Thank you for your patience.
>>
>> Just to clarify, does it include the dependent "CREATE TABLE ( ..
>> STORAGE .. )" patch [1]? I was considering changing the patch
>> according to the feedback it got, but if you are already working on
>> this I'm not going to interfere.
>>
>> [1]: https://postgr.es/m/de83407a-ae3d-a8e1-a788-920eb334f25b%40sigaev.ru
>> --
>> Best regards,
>> Aleksander Alekseev
>
>
Attachments:
[application/x-gzip] 0005_bytea_appendable_toaster_v6.patch.gz (5.6K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/3-0005_bytea_appendable_toaster_v6.patch.gz)
download
[application/x-gzip] 0003_toaster_default_v6.patch.gz (29.9K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/4-0003_toaster_default_v6.patch.gz)
download
[application/x-gzip] 0002_toaster_interface_v7.patch.gz (46.6K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/5-0002_toaster_interface_v7.patch.gz)
download
[application/x-gzip] 0001_create_table_storage_v4.patch.gz (4.2K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/6-0001_create_table_storage_v4.patch.gz)
download
[application/x-gzip] 0004_toaster_snapshot_v6.patch.gz (8.1K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/7-0004_toaster_snapshot_v6.patch.gz)
download
[application/x-gzip] 0006_toasterapi_docs_v1.patch.gz (3.9K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/8-0006_toasterapi_docs_v1.patch.gz)
download
[application/x-gzip] 0008_fix_toast_tuple_externalize_v2.patch.gz (583B, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/9-0008_fix_toast_tuple_externalize_v2.patch.gz)
download
[application/x-gzip] 0006_toasterapi_docs_v2.patch.gz (3.9K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/10-0006_toasterapi_docs_v2.patch.gz)
download
[application/x-gzip] 0009_bytea_contrib_and_varlena_v1.patch.gz (3.9K, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/11-0009_bytea_contrib_and_varlena_v1.patch.gz)
download
[application/x-gzip] 0007_fix_alignment_of_custom_toast_pointers_v2.patch.gz (801B, ../../CAN-LCVOruhrrrK+wNAF=6kPkhWfgik6+MdYZ1yJARf2qfHc-qA@mail.gmail.com/12-0007_fix_alignment_of_custom_toast_pointers_v2.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-06-30 20:27 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
1 sibling, 0 replies; 39+ messages in thread
From: Nikita Malakhov @ 2022-06-30 20:27 UTC (permalink / raw)
To: [email protected]; +Cc: Nikita Malakhov <[email protected]>; Fedor Sigaev <[email protected]>; Nikita Glukhov <[email protected]>; Oleg Bartunov <[email protected]>
Rebased onto 15 REL BETA 2
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-01 12:27 Matthias van de Meent <[email protected]>
parent: Nikita Malakhov <[email protected]>
1 sibling, 1 reply; 39+ messages in thread
From: Matthias van de Meent @ 2022-07-01 12:27 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
On Thu, 30 Jun 2022 at 22:26, Nikita Malakhov <[email protected]> wrote:
>
> Hi hackers!
> Here is the patch set rebased onto current master (15 rel beta 2 with commit from 29.06).
Thanks!
> Just to remind:
> With this patch set you will be able to develop and plug in your own TOAST mechanics for table columns. Knowing internals and/or workflow and workload
> of data being TOASTed makes Custom Toasters much more efficient in performance and storage.
The new toast API doesn't seem to be very well documented, nor are the
new features. Could you include a README or extend the comments on how
this is expected to work, and/or how you expect people to use (the
result of) `get_vtable`?
> Patch set consists of 9 incremental patches:
> [...]
> 0002_toaster_interface_v7.patch - TOAST API interface and SQL syntax allowing creation of custom Toaster (CREATE TOASTER ...)
> and setting Toaster to a table column (CREATE TABLE t (data bytea STORAGE EXTERNAL TOASTER bytea_toaster);)
This patch 0002 seems to include changes to log files (!) that don't
exist in current HEAD, but at the same time are not created by patch
0001. Could you please check and sanitize your patches to ensure that
the changes are actually accurate?
Like Robert Haas mentioned earlier[0], please create a branch in a git
repository that has a commit containing the changes for each patch,
and then use git format-patch to generate a single patchset, one that
shares a single version number. Keeping track of what patches are
needed to test this CF entry is already quite difficult due to the
amount of patches and their packaging (I'm having troubles managing
these seperate .patch.gz), and the different version tags definitely
don't help in finding the correct set of patches to apply once
downloaded.
Kind regards,
Matthias van de Meent
[0] https://www.postgresql.org/message-id/CA%2BTgmoZBgNipyKuQAJzNw2w7C9z%2B2SMC0SAHqCnc_dG1nSLNcw%40mail...
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-11 12:03 Nikita Malakhov <[email protected]>
parent: Matthias van de Meent <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-07-11 12:03 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi!
We have branch with incremental commits worm where patches were generated
with format-patch -
https://github.com/postgrespro/postgres/tree/toasterapi_clean
I'll clean up commits from garbage files asap, sorry, haven't noticed them
while moving changes.
Best regards,
Nikita Malakhov
On Fri, Jul 1, 2022 at 3:27 PM Matthias van de Meent <
[email protected]> wrote:
> On Thu, 30 Jun 2022 at 22:26, Nikita Malakhov <[email protected]> wrote:
> >
> > Hi hackers!
> > Here is the patch set rebased onto current master (15 rel beta 2 with
> commit from 29.06).
>
> Thanks!
>
> > Just to remind:
> > With this patch set you will be able to develop and plug in your own
> TOAST mechanics for table columns. Knowing internals and/or workflow and
> workload
> > of data being TOASTed makes Custom Toasters much more efficient in
> performance and storage.
>
> The new toast API doesn't seem to be very well documented, nor are the
> new features. Could you include a README or extend the comments on how
> this is expected to work, and/or how you expect people to use (the
> result of) `get_vtable`?
>
> > Patch set consists of 9 incremental patches:
> > [...]
> > 0002_toaster_interface_v7.patch - TOAST API interface and SQL syntax
> allowing creation of custom Toaster (CREATE TOASTER ...)
> > and setting Toaster to a table column (CREATE TABLE t (data bytea
> STORAGE EXTERNAL TOASTER bytea_toaster);)
>
> This patch 0002 seems to include changes to log files (!) that don't
> exist in current HEAD, but at the same time are not created by patch
> 0001. Could you please check and sanitize your patches to ensure that
> the changes are actually accurate?
>
> Like Robert Haas mentioned earlier[0], please create a branch in a git
> repository that has a commit containing the changes for each patch,
> and then use git format-patch to generate a single patchset, one that
> shares a single version number. Keeping track of what patches are
> needed to test this CF entry is already quite difficult due to the
> amount of patches and their packaging (I'm having troubles managing
> these seperate .patch.gz), and the different version tags definitely
> don't help in finding the correct set of patches to apply once
> downloaded.
>
> Kind regards,
>
> Matthias van de Meent
>
> [0]
> https://www.postgresql.org/message-id/CA%2BTgmoZBgNipyKuQAJzNw2w7C9z%2B2SMC0SAHqCnc_dG1nSLNcw%40mail...
>
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-13 19:45 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-07-13 19:45 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
According to previous requests the patch branch was cleaned up from
garbage, logs, etc. All conflicts' resolutions were merged
into patch commits where they appear, branch was rebased to present one
commit for one patch. The branch was actualized,
and a fresh patch set was generated.
https://github.com/postgrespro/postgres/tree/toasterapi_clean
What we propose in short:
We suggest a way to make TOAST pluggable as Storage (in a way like
Pluggable Access
Methods) - detached TOAST
mechanics from Heap AM, and made it an independent pluggable and extensible
part with our freshly developed TOAST API.
With this patch set you will be able to develop and plug in your own
TOAST mechanics
for table columns. Knowing internals
and/or workflow and workload of data being TOASTed makes Custom Toasters much
more efficient in performance and storage.
We keep backwards compatibility and default TOAST mechanics works as it
worked previously, working silently with any
Toastable datatype
(and TOASTed values and tables from previous versions, no changes in this)
and set as default Toaster is not stated otherwise,
but through our TOAST API.
We've already presented out work at HighLoad, PgCon and PgConf conferences,
you can find materials here
http://www.sai.msu.su/~megera/postgres/talks/
Testing scripts used in talks are a bit scarce and have a lot of
manual handling, so it is another bit of work to bunch them into
patch set, please be patient, I'll try to make it ASAP.
We have ready to plug in extension Toasters
- bytea appendable toaster for bytea datatype (impressive speedup with
bytea append operation) is included in this patch set;
- JSONB toaster for JSONB (very cool performance improvements when dealing
with TOASTed JSONB) will be provided later;
- Prototype Toasters (in development) for PostGIS (much faster then default
with geometric data), large binary objects
(like pg_largeobject, but much, much larger, and without existing large
object limitations), and currently we're checking default
Toaster implementation without using Indexes (direct access by TIDs, up to
3 times faster than default on smaller values,
less storage due to absence of index tree).
Patch set consists of 8 incremental patches:
0001_create_table_storage_v5.patch - SQL syntax fix for CREATE TABLE
clause, processing SET STORAGE... correctly;
This patch is already discussed in a separate thread;
0002_toaster_interface_v8.patch - TOAST API interface and SQL syntax
allowing creation of custom Toaster (CREATE TOASTER ...)
and setting Toaster to a table column (CREATE TABLE t (data bytea STORAGE
EXTERNAL TOASTER bytea_toaster);)
0003_toaster_default_v7.patch - Default TOAST implemented via TOAST API;
0004_toaster_snapshot_v7.patch - refactoring of Default TOAST and support
for versioned Toast rows;
0005_bytea_appendable_toaster_v7.patch - contrib module
bytea_appendable_toaster - special Toaster for bytea datatype with
customized append operation;
0006_toasterapi_docs_v3.patch - documentation package for Pluggable TOAST;
0007_fix_alignment_of_custom_toast_pointers_v3.patch - fixes custom toast
pointer's
alignment required by bytea toaster by Nikita Glukhov;
0008_fix_toast_tuple_externalize_v3.patch - fixes toast_tuple_externalize
function
not to call toast if old data is the same as new one.
The example of usage the TOAST API:
CREATE EXTENSION bytea_toaster;CREATE TABLE test_bytea_append (id int, a
bytea STORAGE EXTERNAL);
ALTER TABLE test_bytea_append ALTER a SET TOASTER bytea_toaster;
INSERT INTO test_bytea_append SELECT i, repeat('a', 10000)::bytea FROM
generate_series(1, 10) i;
UPDATE test_bytea_append SET a = a || repeat('b', 3000)::bytea;
This patch set opens the following issues:
1) With TOAST independent of AM it is used by it makes sense to move
compression from AM into Toaster and make Compression one of Toaster's
options.
Actually, Toasters allow to use any compression methods independently of AM;
2) Implement default Toaster without using Indexes (currently in
development)?
3) Allows different, SQL-accessed large objects of almost infinite size IN
DATABASE, unlike current large_object functionality and does not limit
their quantity;
4) Several already developed Toasters show impressive results for
datatypes they were designed for.
We're awaiting feedback.
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
On Mon, Jul 11, 2022 at 3:03 PM Nikita Malakhov <[email protected]> wrote:
> Hi!
> We have branch with incremental commits worm where patches were generated
> with format-patch -
> https://github.com/postgrespro/postgres/tree/toasterapi_clean
> I'll clean up commits from garbage files asap, sorry, haven't noticed them
> while moving changes.
>
> Best regards,
> Nikita Malakhov
>
> On Fri, Jul 1, 2022 at 3:27 PM Matthias van de Meent <
> [email protected]> wrote:
>
>> On Thu, 30 Jun 2022 at 22:26, Nikita Malakhov <[email protected]> wrote:
>> >
>> > Hi hackers!
>> > Here is the patch set rebased onto current master (15 rel beta 2 with
>> commit from 29.06).
>>
>> Thanks!
>>
>> > Just to remind:
>> > With this patch set you will be able to develop and plug in your own
>> TOAST mechanics for table columns. Knowing internals and/or workflow and
>> workload
>> > of data being TOASTed makes Custom Toasters much more efficient in
>> performance and storage.
>>
>> The new toast API doesn't seem to be very well documented, nor are the
>> new features. Could you include a README or extend the comments on how
>> this is expected to work, and/or how you expect people to use (the
>> result of) `get_vtable`?
>>
>> > Patch set consists of 9 incremental patches:
>> > [...]
>> > 0002_toaster_interface_v7.patch - TOAST API interface and SQL syntax
>> allowing creation of custom Toaster (CREATE TOASTER ...)
>> > and setting Toaster to a table column (CREATE TABLE t (data bytea
>> STORAGE EXTERNAL TOASTER bytea_toaster);)
>>
>> This patch 0002 seems to include changes to log files (!) that don't
>> exist in current HEAD, but at the same time are not created by patch
>> 0001. Could you please check and sanitize your patches to ensure that
>> the changes are actually accurate?
>>
>> Like Robert Haas mentioned earlier[0], please create a branch in a git
>> repository that has a commit containing the changes for each patch,
>> and then use git format-patch to generate a single patchset, one that
>> shares a single version number. Keeping track of what patches are
>> needed to test this CF entry is already quite difficult due to the
>> amount of patches and their packaging (I'm having troubles managing
>> these seperate .patch.gz), and the different version tags definitely
>> don't help in finding the correct set of patches to apply once
>> downloaded.
>>
>> Kind regards,
>>
>> Matthias van de Meent
>>
>> [0]
>> https://www.postgresql.org/message-id/CA%2BTgmoZBgNipyKuQAJzNw2w7C9z%2B2SMC0SAHqCnc_dG1nSLNcw%40mail...
>>
>
>
>
Attachments:
[application/x-gzip] 0001_create_table_storage_v5.patch.gz (4.2K, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/3-0001_create_table_storage_v5.patch.gz)
download
[application/x-gzip] 0004_toaster_snapshot_v7.patch.gz (7.1K, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/4-0004_toaster_snapshot_v7.patch.gz)
download
[application/x-gzip] 0003_toaster_default_v7.patch.gz (28.4K, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/5-0003_toaster_default_v7.patch.gz)
download
[application/x-gzip] 0002_toaster_interface_v8.patch.gz (44.8K, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/6-0002_toaster_interface_v8.patch.gz)
download
[application/x-gzip] 0005_bytea_appendable_toaster_v7.patch.gz (6.3K, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/7-0005_bytea_appendable_toaster_v7.patch.gz)
download
[application/x-gzip] 0006_toasterapi_docs_v3.patch.gz (3.9K, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/8-0006_toasterapi_docs_v3.patch.gz)
download
[application/x-gzip] 0008_fix_toast_tuple_externalize_v3.patch.gz (584B, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/9-0008_fix_toast_tuple_externalize_v3.patch.gz)
download
[application/x-gzip] 0007_fix_alignment_of_custom_toast_pointers_v3.patch.gz (801B, ../../CAN-LCVNkU+kdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg@mail.gmail.com/10-0007_fix_alignment_of_custom_toast_pointers_v3.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-20 09:15 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-07-20 09:15 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
We really need your feedback on the last patchset update!
On a previous question about TOAST API overhead - please check script in
attach, we tested INSERT, UPDATE and SELECT
operations, and ran it on vanilla master and on patched master (vanilla
with untouched TOAST implementation and patched
with default TOAST implemented via TOAST API, in this patch set - with
patches up to 0005_bytea_appendable_toaster installed).
Some of the test scripts will be included in the patch set later, as an
additional patch.
Currently I'm working on an update to the default Toaster (some internal
optimizations, not affecting functionality)
and readme files explaining Pluggable TOAST.
An example of using custom Toaster:
Custom Toaster extension definition (developer):
CREATE FUNCTION custom_toaster_handler(internal)
RETURNS toaster_handler
AS 'MODULE_PATHNAME'
LANGUAGE C;
CREATE TOASTER custom_toaster HANDLER custom_toaster_handler;
User's POV:
CREATE EXTENSION custom_toaster;
select * from pg_toaster;
oid | tsrname | tsrhandler
-------+----------------+-------------------------
9864 | deftoaster | default_toaster_handler
32772 | custom_toaster | custom_toaster_handler
CREATE TABLE tst1 (
c1 text STORAGE plain,
c2 text STORAGE external TOASTER custom_toaster,
id int4
);
ALTER TABLE tst1 ALTER COLUMN c1 SET TOASTER custom_toaster;
=# \d+ tst1
Column | Type | Collation | Nullable | Default | Storage | Toaster
|...
--------+---------+-----------+----------+---------+----------+----------------+...
c1 | text | | | | plain | deftoaster
|...
c2 | text | | | | external |
custom_toaster |...
id | integer | | | | plain |
|...
Access method: heap
Thanks!
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
On Wed, Jul 13, 2022 at 10:45 PM Nikita Malakhov <[email protected]> wrote:
> Hi hackers!
> According to previous requests the patch branch was cleaned up from
> garbage, logs, etc. All conflicts' resolutions were merged
> into patch commits where they appear, branch was rebased to present one
> commit for one patch. The branch was actualized,
> and a fresh patch set was generated.
> https://github.com/postgrespro/postgres/tree/toasterapi_clean
>
> What we propose in short:
> We suggest a way to make TOAST pluggable as Storage (in a way like
> Pluggable Access Methods) - detached TOAST
> mechanics from Heap AM, and made it an independent pluggable and
> extensible part with our freshly developed TOAST API.
> With this patch set you will be able to develop and plug in your own TOAST mechanics
> for table columns. Knowing internals
> and/or workflow and workload of data being TOASTed makes Custom Toasters much
> more efficient in performance and storage.
> We keep backwards compatibility and default TOAST mechanics works as it
> worked previously, working silently with any
> Toastable datatype
> (and TOASTed values and tables from previous versions, no changes in this)
> and set as default Toaster is not stated otherwise,
> but through our TOAST API.
>
> We've already presented out work at HighLoad, PgCon and PgConf
> conferences, you can find materials here
> http://www.sai.msu.su/~megera/postgres/talks/
> Testing scripts used in talks are a bit scarce and have a lot of
> manual handling, so it is another bit of work to bunch them into
> patch set, please be patient, I'll try to make it ASAP.
>
> We have ready to plug in extension Toasters
> - bytea appendable toaster for bytea datatype (impressive speedup with
> bytea append operation) is included in this patch set;
> - JSONB toaster for JSONB (very cool performance improvements when
> dealing with TOASTed JSONB) will be provided later;
> - Prototype Toasters (in development) for PostGIS (much faster then
> default with geometric data), large binary objects
> (like pg_largeobject, but much, much larger, and without existing large
> object limitations), and currently we're checking default
> Toaster implementation without using Indexes (direct access by TIDs, up
> to 3 times faster than default on smaller values,
> less storage due to absence of index tree).
>
> Patch set consists of 8 incremental patches:
> 0001_create_table_storage_v5.patch - SQL syntax fix for CREATE TABLE
> clause, processing SET STORAGE... correctly;
> This patch is already discussed in a separate thread;
>
> 0002_toaster_interface_v8.patch - TOAST API interface and SQL syntax
> allowing creation of custom Toaster (CREATE TOASTER ...)
> and setting Toaster to a table column (CREATE TABLE t (data bytea STORAGE
> EXTERNAL TOASTER bytea_toaster);)
>
> 0003_toaster_default_v7.patch - Default TOAST implemented via TOAST API;
>
> 0004_toaster_snapshot_v7.patch - refactoring of Default TOAST and support
> for versioned Toast rows;
>
> 0005_bytea_appendable_toaster_v7.patch - contrib module
> bytea_appendable_toaster - special Toaster for bytea datatype with
> customized append operation;
>
> 0006_toasterapi_docs_v3.patch - documentation package for Pluggable TOAST;
>
> 0007_fix_alignment_of_custom_toast_pointers_v3.patch - fixes custom toast
> pointer's
> alignment required by bytea toaster by Nikita Glukhov;
>
> 0008_fix_toast_tuple_externalize_v3.patch - fixes toast_tuple_externalize
> function
> not to call toast if old data is the same as new one.
>
> The example of usage the TOAST API:
> CREATE EXTENSION bytea_toaster;CREATE TABLE test_bytea_append (id int, a
> bytea STORAGE EXTERNAL);
> ALTER TABLE test_bytea_append ALTER a SET TOASTER bytea_toaster;
> INSERT INTO test_bytea_append SELECT i, repeat('a', 10000)::bytea FROM
> generate_series(1, 10) i;
> UPDATE test_bytea_append SET a = a || repeat('b', 3000)::bytea;
>
> This patch set opens the following issues:
> 1) With TOAST independent of AM it is used by it makes sense to move
> compression from AM into Toaster and make Compression one of Toaster's
> options.
> Actually, Toasters allow to use any compression methods independently of
> AM;
> 2) Implement default Toaster without using Indexes (currently in
> development)?
> 3) Allows different, SQL-accessed large objects of almost infinite size IN
> DATABASE, unlike current large_object functionality and does not limit
> their quantity;
> 4) Several already developed Toasters show impressive results for
> datatypes they were designed for.
>
> We're awaiting feedback.
>
> Regards,
> Nikita Malakhov
> Postgres Professional
> https://postgrespro.ru/
>
> On Mon, Jul 11, 2022 at 3:03 PM Nikita Malakhov <[email protected]> wrote:
>
>> Hi!
>> We have branch with incremental commits worm where patches were generated
>> with format-patch -
>> https://github.com/postgrespro/postgres/tree/toasterapi_clean
>> I'll clean up commits from garbage files asap, sorry, haven't noticed
>> them while moving changes.
>>
>> Best regards,
>> Nikita Malakhov
>>
>> On Fri, Jul 1, 2022 at 3:27 PM Matthias van de Meent <
>> [email protected]> wrote:
>>
>>> On Thu, 30 Jun 2022 at 22:26, Nikita Malakhov <[email protected]> wrote:
>>> >
>>> > Hi hackers!
>>> > Here is the patch set rebased onto current master (15 rel beta 2 with
>>> commit from 29.06).
>>>
>>> Thanks!
>>>
>>> > Just to remind:
>>> > With this patch set you will be able to develop and plug in your own
>>> TOAST mechanics for table columns. Knowing internals and/or workflow and
>>> workload
>>> > of data being TOASTed makes Custom Toasters much more efficient in
>>> performance and storage.
>>>
>>> The new toast API doesn't seem to be very well documented, nor are the
>>> new features. Could you include a README or extend the comments on how
>>> this is expected to work, and/or how you expect people to use (the
>>> result of) `get_vtable`?
>>>
>>> > Patch set consists of 9 incremental patches:
>>> > [...]
>>> > 0002_toaster_interface_v7.patch - TOAST API interface and SQL syntax
>>> allowing creation of custom Toaster (CREATE TOASTER ...)
>>> > and setting Toaster to a table column (CREATE TABLE t (data bytea
>>> STORAGE EXTERNAL TOASTER bytea_toaster);)
>>>
>>> This patch 0002 seems to include changes to log files (!) that don't
>>> exist in current HEAD, but at the same time are not created by patch
>>> 0001. Could you please check and sanitize your patches to ensure that
>>> the changes are actually accurate?
>>>
>>> Like Robert Haas mentioned earlier[0], please create a branch in a git
>>> repository that has a commit containing the changes for each patch,
>>> and then use git format-patch to generate a single patchset, one that
>>> shares a single version number. Keeping track of what patches are
>>> needed to test this CF entry is already quite difficult due to the
>>> amount of patches and their packaging (I'm having troubles managing
>>> these seperate .patch.gz), and the different version tags definitely
>>> don't help in finding the correct set of patches to apply once
>>> downloaded.
>>>
>>> Kind regards,
>>>
>>> Matthias van de Meent
>>>
>>> [0]
>>> https://www.postgresql.org/message-id/CA%2BTgmoZBgNipyKuQAJzNw2w7C9z%2B2SMC0SAHqCnc_dG1nSLNcw%40mail...
>>>
>>
>>
>>
>
>
Attachments:
[application/octet-stream] api_perf.sql (2.8K, ../../CAN-LCVNMot2xqq2pHi4OCbgA1nK4KkRfWiJQGK9EgiTbW3O8gQ@mail.gmail.com/3-api_perf.sql)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-22 13:16 Matthias van de Meent <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Matthias van de Meent @ 2022-07-22 13:16 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
On Wed, 20 Jul 2022 at 11:16, Nikita Malakhov <[email protected]> wrote:
>
> Hi hackers!
Hi,
Please don't top-post here. See
https://wiki.postgresql.org/wiki/Mailing_Lists#Email_etiquette_mechanics.
> We really need your feedback on the last patchset update!
This is feedback on the latest version that was shared on the mailing
list here [0]. Your mail from today didn't seem to have an attachment,
and I haven't checked the git repository for changes.
0001: Create Table Storage:
LGTM
---
0002: Toaster API interface
> tablecmds.c
> SetIndexStorageProperties(Relation rel, Relation attrelation,
> AttrNumber attnum,
> bool setstorage, char newstorage,
> bool setcompression, char newcompression,
> + bool settoaster, Oid toasterOid,
> LOCKMODE lockmode)
Indexes cannot index toasted values, so why would the toaster oid be
interesting for index storage properties?
> static List *MergeAttributes(List *schema, List *supers, char relpersistence,
> - bool is_partition, List **supconstr);
> + bool is_partition, List **supconstr,
> + Oid accessMethodId);
> toasterapi.h:
> +SearchTsrCache(Oid toasterOid)
> ...
> + for_each_from(lc, ToasterCache, 0)
> + {
> + entry = (ToasterCacheEntry*)lfirst(lc);
> +
> + if (entry->toasterOid == toasterOid)
> + {
> + /* remove entry from list, it will be added in a head of list below */
> + foreach_delete_current(ToasterCache, lc);
> + goto out;
> + }
> + }
Moving toasters seems quite expensive when compared to just index
checks. When you have many toasters, but only a few hot ones, this
currently will move the "cold" toasters around a lot. Why not use a
stack instead (or alternatively, a 'zipper' (or similar) data
structure), where the hottest toaster is on top, so that we avoid
larger memmove calls?
> postgres.h
> +/* varatt_custom uses 16bit aligment */
To the best of my knowledge varlena-pointers are unaligned; and this
is affirmed by the comment immediately under varattrib_1b_e. Assuming
alignment to 16 bits should/would be incorrect in some of the cases.
This is handled for normal varatt_external values by memcpy-ing the
value into local (aligned) fields before using them, but that doesn't
seem to be the case for varatt_custom?
---
0003: (re-implement default toaster using toaster interface)
I see significant changes to the dummy toaster (created in 0002),
could those be moved to patch 0002 in the next iteration?
detoast.c and detoast.h are effectively empty after this patch (only
imports and commented-out code remain), please fully remove them
instead - that saves on patch diff size.
With the new API, I'm getting confused about the location of the
various toast_* functions. They are spread around in various files
that have no clear distinction on why it is (still) located there:
some functions are moved to access/toast/*, while others are moved
around in catalog/toasting.c, access/common/toast_internals.c and
access/table/toast_helper.c.
> detoast.c / tableam.h
According to a quick search, all core usage of
table_relation_fetch_toast_slice is removed. Shouldn't we remove that
tableAM API (+ heapam implementation) instead of updating and
maintaining it? Same question for table_relation_toast_am - I'm not
sure that it remains the correct way of dealing with toast.
> toast_helper.c
toast_delete_external_datum:
Please clean up code that was commented out from the patches, it
detracts from the readability of a patch.
> toast_internals.c
This seems like a bit of a mess, considering the lack of
Can't we split this up into a heaptoast (or whatever we're going to
call the default toaster) and actual toast internals? It seems to me
that
> generic_toaster.c
Could you align name styles in this new file? It has both camelCase
and snake_case for function names.
> toasting.c
I'm not entirely sure that we should retain catalog/toasting.c if we
are going to depend on the custom toaster API. Shouldn't the creation
of toast tables be delegated to the toaster?
> + * toast_get_valid_index
> + *
> + * Get OID of valid index associated to given toast relation. A toast
> + * relation can have only one valid index at the same time.
Although this is code being moved around, the comment is demonstrably
false: A cancelled REINDEX CONCURRENTLY with a subsequent REINDEX can
leave a toast relation with 2 valid indexes.
---
0004: refactoring and optimization of default toaster
0005: bytea appendable toaster
I dind't review these yet.
---
0006: docs
Seems like a good start, but I'm not sure that we need the struct
definition in the docs. I think the BRIN extensibility docs [1] are a
better example on what I think the documentation for this API should
look like.
---
0007: fix alignment of custom toast pointers
This is not a valid fix for the alignment requirement for custom toast
pointers. You now leave one byte empty if you are not already aligned,
which for on-disk toast pointers means that we're dealing with a
4-byte aligned value, which is not the case because this is a
2-byte-aligned value.
Regardless, this should be merged into 0002, not remain a seperate patch.
---
0008: fix tuple externalization
Should be merged into the relevant patch as well, not as a separate patch.
---
> Currently I'm working on an update to the default Toaster (some internal optimizations, not affecting functionality)
> and readme files explaining Pluggable TOAST.
That would be greatly appreciated - 0006 does not cover why we need
vtable, nor how it's expected to be used in type-aware code.
Kind regards,
Matthias van de Meent
[0] https://www.postgresql.org/message-id/CAN-LCVNkU%2Bkdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg%40mail.g...
[1] https://www.postgresql.org/docs/15/brin-extensibility.html
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-23 07:15 Nikita Malakhov <[email protected]>
parent: Matthias van de Meent <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-07-23 07:15 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
Matthias, thank you very much for your feedback!
Sorry, I forgot to attach files.
Attaching here, but they are for the commit tagged "15beta2", I am
currently
rebasing this branch onto the actual master and will provide rebased
version,
with some corrections according to your feedback, in a day or two.
>Indexes cannot index toasted values, so why would the toaster oid be
>interesting for index storage properties?
Here Teodor might correct me. Toast tables are indexed, and Heap TOAST
mechanics accesses Toasted tuples by index, isn't it the case?
>Moving toasters seems quite expensive when compared to just index
>checks. When you have many toasters, but only a few hot ones, this
>currently will move the "cold" toasters around a lot. Why not use a
>stack instead (or alternatively, a 'zipper' (or similar) data
>structure), where the hottest toaster is on top, so that we avoid
>larger memmove calls?
This is a reasonable remark, we'll consider it for the next iteration. Our
reason
is that we think there won't be a lot of custom Toasters, most likely less
then
a dozen, for the most complex/heavy datatypes so we haven't considered
these expenses.
>To the best of my knowledge varlena-pointers are unaligned; and this
>is affirmed by the comment immediately under varattrib_1b_e. Assuming
>alignment to 16 bits should/would be incorrect in some of the cases.
>This is handled for normal varatt_external values by memcpy-ing the
>value into local (aligned) fields before using them, but that doesn't
>seem to be the case for varatt_custom?
Alignment correction seemed reasonable for us because structures are
anyway aligned in memory, so when we use 1 and 2-byte fields along
with 4-byte it may create a lot of padding. Am I wrong? Also, correct
alignment somewhat simplifies access to structure fields.
>0003: (re-implement default toaster using toaster interface)
>I see significant changes to the dummy toaster (created in 0002),
>could those be moved to patch 0002 in the next iteration?
Will do.
>detoast.c and detoast.h are effectively empty after this patch (only
>imports and commented-out code remain), please fully remove them
>instead - that saves on patch diff size.
Will do.
About the location of toast_ functions: these functions are part of Heap
TOAST mechanics, and they were scattered among other Heap internals
sources. I've tried to gather them and put them in more straight order, but
this work is not fully finished yet and will take some time. Working on it.
I'll check if table_relation_fetch_toast_slice could be removed, thanks for
the remark.
toast_helper - done, will be provided in rebased version.
toast_internals - this one is an internal part of TOAST implemented in
Heap AM, but I'll try to straighten it out as much as I could.
naming conventions in some sources - done, will be provided in rebased
patch set.
>Shouldn't the creation of toast tables be delegated to the toaster?
Yes, you're right, and actually, it is. I'll check that and correct in
rebased
version.
>Although this is code being moved around, the comment is demonstrably
>false: A cancelled REINDEX CONCURRENTLY with a subsequent REINDEX can
>leave a toast relation with 2 valid indexes.
This code is quite old, we've not changed it but thanks for the remark,
I'll check it more carefully.
Small fixes are already merged into larger patches in attached files. Also,
I appreciate your feedback on documentation - if you would have an
opportunity
please check README provided in 0003. I've took your comments on
documentation
into account and will include corrections according to them into rebased
patch.
As Aleksander recommended, I've shortened the patch set and left only the
most
important part - API and re-implemented default Toast. All bells and
whistles are not
of so much importance and could be sent later after the API itself will be
straightened
out and commited.
Thank you very much!
On Fri, Jul 22, 2022 at 4:17 PM Matthias van de Meent <
[email protected]> wrote:
> On Wed, 20 Jul 2022 at 11:16, Nikita Malakhov <[email protected]> wrote:
> >
> > Hi hackers!
>
> Hi,
>
> Please don't top-post here. See
> https://wiki.postgresql.org/wiki/Mailing_Lists#Email_etiquette_mechanics.
>
> > We really need your feedback on the last patchset update!
>
> This is feedback on the latest version that was shared on the mailing
> list here [0]. Your mail from today didn't seem to have an attachment,
> and I haven't checked the git repository for changes.
>
> 0001: Create Table Storage:
> LGTM
>
> ---
>
> 0002: Toaster API interface
>
> > tablecmds.c
>
> > SetIndexStorageProperties(Relation rel, Relation attrelation,
> > AttrNumber attnum,
> > bool setstorage, char newstorage,
> > bool setcompression, char newcompression,
> > + bool settoaster, Oid toasterOid,
> > LOCKMODE lockmode)
>
> Indexes cannot index toasted values, so why would the toaster oid be
> interesting for index storage properties?
>
> > static List *MergeAttributes(List *schema, List *supers, char
> relpersistence,
> > - bool is_partition, List **supconstr);
> > + bool is_partition, List **supconstr,
> > + Oid accessMethodId);
>
> > toasterapi.h:
>
> > +SearchTsrCache(Oid toasterOid)
> > ...
> > + for_each_from(lc, ToasterCache, 0)
> > + {
> > + entry = (ToasterCacheEntry*)lfirst(lc);
> > +
> > + if (entry->toasterOid == toasterOid)
> > + {
> > + /* remove entry from list, it will be added in a head of
> list below */
> > + foreach_delete_current(ToasterCache, lc);
> > + goto out;
> > + }
> > + }
>
> Moving toasters seems quite expensive when compared to just index
> checks. When you have many toasters, but only a few hot ones, this
> currently will move the "cold" toasters around a lot. Why not use a
> stack instead (or alternatively, a 'zipper' (or similar) data
> structure), where the hottest toaster is on top, so that we avoid
> larger memmove calls?
>
> > postgres.h
>
> > +/* varatt_custom uses 16bit aligment */
> To the best of my knowledge varlena-pointers are unaligned; and this
> is affirmed by the comment immediately under varattrib_1b_e. Assuming
> alignment to 16 bits should/would be incorrect in some of the cases.
> This is handled for normal varatt_external values by memcpy-ing the
> value into local (aligned) fields before using them, but that doesn't
> seem to be the case for varatt_custom?
>
> ---
>
> 0003: (re-implement default toaster using toaster interface)
>
> I see significant changes to the dummy toaster (created in 0002),
> could those be moved to patch 0002 in the next iteration?
>
> detoast.c and detoast.h are effectively empty after this patch (only
> imports and commented-out code remain), please fully remove them
> instead - that saves on patch diff size.
>
> With the new API, I'm getting confused about the location of the
> various toast_* functions. They are spread around in various files
> that have no clear distinction on why it is (still) located there:
> some functions are moved to access/toast/*, while others are moved
> around in catalog/toasting.c, access/common/toast_internals.c and
> access/table/toast_helper.c.
>
> > detoast.c / tableam.h
> According to a quick search, all core usage of
> table_relation_fetch_toast_slice is removed. Shouldn't we remove that
> tableAM API (+ heapam implementation) instead of updating and
> maintaining it? Same question for table_relation_toast_am - I'm not
> sure that it remains the correct way of dealing with toast.
>
> > toast_helper.c
>
> toast_delete_external_datum:
> Please clean up code that was commented out from the patches, it
> detracts from the readability of a patch.
>
> > toast_internals.c
>
> This seems like a bit of a mess, considering the lack of
> Can't we split this up into a heaptoast (or whatever we're going to
> call the default toaster) and actual toast internals? It seems to me
> that
>
> > generic_toaster.c
>
> Could you align name styles in this new file? It has both camelCase
> and snake_case for function names.
>
> > toasting.c
>
> I'm not entirely sure that we should retain catalog/toasting.c if we
> are going to depend on the custom toaster API. Shouldn't the creation
> of toast tables be delegated to the toaster?
>
> > + * toast_get_valid_index
> > + *
> > + * Get OID of valid index associated to given toast relation. A toast
> > + * relation can have only one valid index at the same time.
>
> Although this is code being moved around, the comment is demonstrably
> false: A cancelled REINDEX CONCURRENTLY with a subsequent REINDEX can
> leave a toast relation with 2 valid indexes.
>
> ---
>
> 0004: refactoring and optimization of default toaster
> 0005: bytea appendable toaster
>
> I dind't review these yet.
>
> ---
>
> 0006: docs
>
> Seems like a good start, but I'm not sure that we need the struct
> definition in the docs. I think the BRIN extensibility docs [1] are a
> better example on what I think the documentation for this API should
> look like.
>
> ---
>
> 0007: fix alignment of custom toast pointers
> This is not a valid fix for the alignment requirement for custom toast
> pointers. You now leave one byte empty if you are not already aligned,
> which for on-disk toast pointers means that we're dealing with a
> 4-byte aligned value, which is not the case because this is a
> 2-byte-aligned value.
>
> Regardless, this should be merged into 0002, not remain a seperate patch.
>
> ---
>
> 0008: fix tuple externalization
> Should be merged into the relevant patch as well, not as a separate patch.
>
> ---
>
> > Currently I'm working on an update to the default Toaster (some internal
> optimizations, not affecting functionality)
> > and readme files explaining Pluggable TOAST.
>
> That would be greatly appreciated - 0006 does not cover why we need
> vtable, nor how it's expected to be used in type-aware code.
>
>
>
> Kind regards,
>
> Matthias van de Meent
>
> [0]
> https://www.postgresql.org/message-id/CAN-LCVNkU%2Bkdieu4i_BDnLgGszNY1RCnL6Dsrdz44fY7FOG3vg%40mail.g...
> [1] https://www.postgresql.org/docs/15/brin-extensibility.html
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 0001_create_table_storage_v6.patch.gz (4.3K, ../../CAN-LCVMBe-U1+phVR21px_kG0rL-5pwz3U2TmCmpM8XtEbPUDg@mail.gmail.com/3-0001_create_table_storage_v6.patch.gz)
download
[application/x-gzip] 0003_toaster_default_v8.patch.gz (33.7K, ../../CAN-LCVMBe-U1+phVR21px_kG0rL-5pwz3U2TmCmpM8XtEbPUDg@mail.gmail.com/4-0003_toaster_default_v8.patch.gz)
download
[application/x-gzip] 0002_toaster_interface_v9.patch.gz (44.8K, ../../CAN-LCVMBe-U1+phVR21px_kG0rL-5pwz3U2TmCmpM8XtEbPUDg@mail.gmail.com/5-0002_toaster_interface_v9.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-25 21:20 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-07-25 21:20 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
Here is the patch set rebased to master from 22.07. I've got some trouble
rebasing it due to
conflicts, so it took some time.
I've made some corrections according to Matthias and Aleksander comments,
though not all
of them, because some require refactoring of very old code and would have
to take much more
effort. I keep these recommended corrections in mind and already working on
them but it will
require extensive testing and some more work, so the will be presented
later, in next iteration
or next patch - these are optimization of heap AM according
table_relation_fetch_toast_slice,
double-index problem and I'm continue to straighten out code related to
TOAST functionality.
It's quite a task because as I mentioned before, this core was scattered
over Heap AM and
reference implementation of TOAST is very tightly intertwined with Heap AM
itself. Default
toaster uses Heap AM storage so it is unlikely that it will be possible to
fully detach it from
Heap.
However, I've made some more refactoring, removed empty sources, corrected
code according
to naming conventions, and extended README.toastapi document.
0002_toaster_interface_v10 contains TOAST API with Dummy toaster as an
example (but I would,
as recommended, remove Dummy toaster and provide it as an extension), and
default Toaster
was left as-is (reference implementation).
0003_toaster_default_v9 implements reference TOAST as Default Toaster via
TOAST API,
so Heap AM calls Toast only via API, and does not have direct calls to
Toast functionality.
0004_toaster_snapshot_v8 continues refactoring and has some important
changes (added
into README.toastapi new part related TOAST API extensibility - the virtual
functions table).
Also, I'll provide documentation package corrected according to Matthias'
remarks later,
in the next patch set.
Please check attached patch set.
Also, GIT branch with this patch resides here:
https://github.com/postgrespro/postgres/tree/toasterapi_clean
Thank all reviewers for feedback!
On Sat, Jul 23, 2022 at 10:15 AM Nikita Malakhov <[email protected]> wrote:
> Hi hackers!
>
> Matthias, thank you very much for your feedback!
> Sorry, I forgot to attach files.
> Attaching here, but they are for the commit tagged "15beta2", I am
> currently
> rebasing this branch onto the actual master and will provide rebased
> version,
> with some corrections according to your feedback, in a day or two.
>
> >Indexes cannot index toasted values, so why would the toaster oid be
> >interesting for index storage properties?
>
> Here Teodor might correct me. Toast tables are indexed, and Heap TOAST
> mechanics accesses Toasted tuples by index, isn't it the case?
>
> >Moving toasters seems quite expensive when compared to just index
> >checks. When you have many toasters, but only a few hot ones, this
> >currently will move the "cold" toasters around a lot. Why not use a
> >stack instead (or alternatively, a 'zipper' (or similar) data
> >structure), where the hottest toaster is on top, so that we avoid
> >larger memmove calls?
>
> This is a reasonable remark, we'll consider it for the next iteration. Our
> reason
> is that we think there won't be a lot of custom Toasters, most likely less
> then
> a dozen, for the most complex/heavy datatypes so we haven't considered
> these expenses.
>
> >To the best of my knowledge varlena-pointers are unaligned; and this
> >is affirmed by the comment immediately under varattrib_1b_e. Assuming
> >alignment to 16 bits should/would be incorrect in some of the cases.
> >This is handled for normal varatt_external values by memcpy-ing the
> >value into local (aligned) fields before using them, but that doesn't
> >seem to be the case for varatt_custom?
>
> Alignment correction seemed reasonable for us because structures are
> anyway aligned in memory, so when we use 1 and 2-byte fields along
> with 4-byte it may create a lot of padding. Am I wrong? Also, correct
> alignment somewhat simplifies access to structure fields.
>
> >0003: (re-implement default toaster using toaster interface)
> >I see significant changes to the dummy toaster (created in 0002),
> >could those be moved to patch 0002 in the next iteration?
> Will do.
>
> >detoast.c and detoast.h are effectively empty after this patch (only
> >imports and commented-out code remain), please fully remove them
> >instead - that saves on patch diff size.
> Will do.
>
> About the location of toast_ functions: these functions are part of Heap
> TOAST mechanics, and they were scattered among other Heap internals
> sources. I've tried to gather them and put them in more straight order,
> but
> this work is not fully finished yet and will take some time. Working on it.
>
> I'll check if table_relation_fetch_toast_slice could be removed, thanks for
> the remark.
>
> toast_helper - done, will be provided in rebased version.
>
> toast_internals - this one is an internal part of TOAST implemented in
> Heap AM, but I'll try to straighten it out as much as I could.
>
> naming conventions in some sources - done, will be provided in rebased
> patch set.
>
> >Shouldn't the creation of toast tables be delegated to the toaster?
>
> Yes, you're right, and actually, it is. I'll check that and correct in
> rebased
> version.
>
> >Although this is code being moved around, the comment is demonstrably
> >false: A cancelled REINDEX CONCURRENTLY with a subsequent REINDEX can
> >leave a toast relation with 2 valid indexes.
>
> This code is quite old, we've not changed it but thanks for the remark,
> I'll check it more carefully.
>
> Small fixes are already merged into larger patches in attached files. Also,
> I appreciate your feedback on documentation - if you would have an
> opportunity
> please check README provided in 0003. I've took your comments on
> documentation
> into account and will include corrections according to them into rebased
> patch.
>
> As Aleksander recommended, I've shortened the patch set and left only the
> most
> important part - API and re-implemented default Toast. All bells and
> whistles are not
> of so much importance and could be sent later after the API itself will be
> straightened
> out and commited.
>
> Thank you very much!
>
> --
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] 0004_toaster_snapshot_v8.patch.gz (9.2K, ../../CAN-LCVOUPnNFV_AAtFsE9oKiWzVji8AF814DBh+C_9e_5ST+AQ@mail.gmail.com/3-0004_toaster_snapshot_v8.patch.gz)
download
[application/x-gzip] 0003_toaster_default_v9.patch.gz (34.0K, ../../CAN-LCVOUPnNFV_AAtFsE9oKiWzVji8AF814DBh+C_9e_5ST+AQ@mail.gmail.com/4-0003_toaster_default_v9.patch.gz)
download
[application/x-gzip] 0002_toaster_interface_v10.patch.gz (44.1K, ../../CAN-LCVOUPnNFV_AAtFsE9oKiWzVji8AF814DBh+C_9e_5ST+AQ@mail.gmail.com/5-0002_toaster_interface_v10.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-26 08:22 Aleksander Alekseev <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Aleksander Alekseev @ 2022-07-26 08:22 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Nikita Malakhov <[email protected]>; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi Nikita,
Thanks for an update!
> 0002_toaster_interface_v10 contains TOAST API with Dummy toaster as an example (but I would,
> as recommended, remove Dummy toaster and provide it as an extension), and default Toaster
> was left as-is (reference implementation).
>
> 0003_toaster_default_v9 implements reference TOAST as Default Toaster via TOAST API,
> so Heap AM calls Toast only via API, and does not have direct calls to Toast functionality.
>
> 0004_toaster_snapshot_v8 continues refactoring and has some important changes (added
> into README.toastapi new part related TOAST API extensibility - the virtual functions table).
This numbering is confusing. Please use a command like:
```
git format-patch origin/master -v 42
```
This will produce a patchset with a consistent naming like:
```
v42-0001-foo-bar.patch
v42-0002-baz-qux.patch
... etc ...
```
Also cfbot [1] will know in which order to apply them.
> GIT branch with this patch resides here:
> https://github.com/postgrespro/postgres/tree/toasterapi_clean
Unfortunately the three patches in question from this branch don't
pass `make check`. Please update
src/test/regress/expected/publication.out and make sure the patchset
passes the rest of the tests at least on one platform before
submitting.
Personally I have a little set of scripts for this [2]. The following
commands should pass:
```
# quick check
./quick-build.sh && ./single-install.sh && make installcheck
# full check
./full-build.sh && ./single-install.sh && make installcheck-world
```
Finally, please update the commit messages. Each commit message should
include a brief description (one line) , a detailed description (the
body), and also the list of the authors, the reviewers and a link to
the discussion. Please use [3] as a template.
[1]: http://cfbot.cputube.org/
[2]: https://github.com/afiskon/pgscripts/
[3]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=784cedda0604ee4ac731fd0b00cd8b27e78c0...
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-07-29 06:16 Nikita Malakhov <[email protected]>
parent: Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-07-29 06:16 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
Aleksander, thanks for the remark, seems that we've missed recent change -
the pubication
test does not have the new column 'Toaster'. Will send a corrected patch
tomorrow. Also, thanks
for the patch name note, I've changed it as you suggested.
I'm on vacation, so I read emails not very often and answers take some
time, sorry.
On Tue, Jul 26, 2022 at 11:23 AM Aleksander Alekseev <
[email protected]> wrote:
> Hi Nikita,
>
> Thanks for an update!
>
> > 0002_toaster_interface_v10 contains TOAST API with Dummy toaster as an
> example (but I would,
> > as recommended, remove Dummy toaster and provide it as an extension),
> and default Toaster
> > was left as-is (reference implementation).
> >
> > 0003_toaster_default_v9 implements reference TOAST as Default Toaster
> via TOAST API,
> > so Heap AM calls Toast only via API, and does not have direct calls to
> Toast functionality.
> >
> > 0004_toaster_snapshot_v8 continues refactoring and has some important
> changes (added
> > into README.toastapi new part related TOAST API extensibility - the
> virtual functions table).
>
> This numbering is confusing. Please use a command like:
>
> ```
> git format-patch origin/master -v 42
> ```
>
> This will produce a patchset with a consistent naming like:
>
> ```
> v42-0001-foo-bar.patch
> v42-0002-baz-qux.patch
> ... etc ...
> ```
>
> Also cfbot [1] will know in which order to apply them.
>
> > GIT branch with this patch resides here:
> > https://github.com/postgrespro/postgres/tree/toasterapi_clean
>
> Unfortunately the three patches in question from this branch don't
> pass `make check`. Please update
> src/test/regress/expected/publication.out and make sure the patchset
> passes the rest of the tests at least on one platform before
> submitting.
>
> Personally I have a little set of scripts for this [2]. The following
> commands should pass:
>
> ```
> # quick check
> ./quick-build.sh && ./single-install.sh && make installcheck
>
> # full check
> ./full-build.sh && ./single-install.sh && make installcheck-world
> ```
>
> Finally, please update the commit messages. Each commit message should
> include a brief description (one line) , a detailed description (the
> body), and also the list of the authors, the reviewers and a link to
> the discussion. Please use [3] as a template.
>
> [1]: http://cfbot.cputube.org/
> [2]: https://github.com/afiskon/pgscripts/
> [3]:
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=784cedda0604ee4ac731fd0b00cd8b27e78c0...
>
> --
> Best regards,
> Aleksander Alekseev
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-08-02 06:15 Nikita Malakhov <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-08-02 06:15 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
Sorry for the delay.
I've made changes according to Aleksander comments:
>This will produce a patchset with a consistent naming like:
>...
>Also cfbot [1] will know in which order to apply them.
Done.
>Unfortunately the three patches in question from this branch don't
>pass `make check`. Please update
>src/test/regress/expected/publication.out and make sure the patchset
>passes the rest of the tests at least on one platform before
>submitting.
Done, there was absent column in Publication tests.
>Finally, please update the commit messages. Each commit message should
>include a brief description (one line) , a detailed description (the
>body), and also the list of the authors, the reviewers and a link to
>the discussion. Please use [3] as a template.
Done.
Link to the rebased branch:
https://github.com/postgrespro/postgres/tree/toasterapi_clean
Please check.
Attach includes:
v11-0002-toaster-interface.patch - contains TOAST API with default Toaster
left as-is (reference implementation) and Dummy toaster as an example
(will be removed later as a part of refactoring?).
v11-0003-toaster-default.patch - implements reference TOAST as Default
Toaster
via TOAST API, so Heap AM calls Toast only via API, and does not have direct
calls to Toast functionality.
v11-0004-toaster-snapshot.patch - supports row versioning for TOASTed values
and some refactoring.
Aleksander, thank you and Matthias for the very helpful feedback!
On Fri, Jul 29, 2022 at 9:16 AM Nikita Malakhov <[email protected]> wrote:
> Hi hackers!
>
> Aleksander, thanks for the remark, seems that we've missed recent change -
> the pubication
> test does not have the new column 'Toaster'. Will send a corrected patch
> tomorrow. Also, thanks
> for the patch name note, I've changed it as you suggested.
> I'm on vacation, so I read emails not very often and answers take some
> time, sorry.
>
>
> On Tue, Jul 26, 2022 at 11:23 AM Aleksander Alekseev <
> [email protected]> wrote:
>
>> Hi Nikita,
>>
>> Thanks for an update!
>>
>> > 0002_toaster_interface_v10 contains TOAST API with Dummy toaster as an
>> example (but I would,
>> > as recommended, remove Dummy toaster and provide it as an extension),
>> and default Toaster
>> > was left as-is (reference implementation).
>> >
>> > 0003_toaster_default_v9 implements reference TOAST as Default Toaster
>> via TOAST API,
>> > so Heap AM calls Toast only via API, and does not have direct calls to
>> Toast functionality.
>> >
>> > 0004_toaster_snapshot_v8 continues refactoring and has some important
>> changes (added
>> > into README.toastapi new part related TOAST API extensibility - the
>> virtual functions table).
>>
>> This numbering is confusing. Please use a command like:
>>
>> ```
>> git format-patch origin/master -v 42
>> ```
>>
>> This will produce a patchset with a consistent naming like:
>>
>> ```
>> v42-0001-foo-bar.patch
>> v42-0002-baz-qux.patch
>> ... etc ...
>> ```
>>
>> Also cfbot [1] will know in which order to apply them.
>>
>> > GIT branch with this patch resides here:
>> > https://github.com/postgrespro/postgres/tree/toasterapi_clean
>>
>> Unfortunately the three patches in question from this branch don't
>> pass `make check`. Please update
>> src/test/regress/expected/publication.out and make sure the patchset
>> passes the rest of the tests at least on one platform before
>> submitting.
>>
>> Personally I have a little set of scripts for this [2]. The following
>> commands should pass:
>>
>> ```
>> # quick check
>> ./quick-build.sh && ./single-install.sh && make installcheck
>>
>> # full check
>> ./full-build.sh && ./single-install.sh && make installcheck-world
>> ```
>>
>> Finally, please update the commit messages. Each commit message should
>> include a brief description (one line) , a detailed description (the
>> body), and also the list of the authors, the reviewers and a link to
>> the discussion. Please use [3] as a template.
>>
>> [1]: http://cfbot.cputube.org/
>> [2]: https://github.com/afiskon/pgscripts/
>> [3]:
>> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=784cedda0604ee4ac731fd0b00cd8b27e78c0...
>>
>> --
>> Best regards,
>> Aleksander Alekseev
>>
>
>
> --
> Regards,
> Nikita Malakhov
> Postgres Professional
> https://postgrespro.ru/
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] v11-0006-pluggable-toast-docs.patch.gz (4.0K, ../../CAN-LCVNLfu-1n94CnZTeD7stToJ=xXGJhrENHjqONZ3n79RRqQ@mail.gmail.com/3-v11-0006-pluggable-toast-docs.patch.gz)
download
[application/x-gzip] v11-0004-toaster-snapshot.patch.gz (9.6K, ../../CAN-LCVNLfu-1n94CnZTeD7stToJ=xXGJhrENHjqONZ3n79RRqQ@mail.gmail.com/4-v11-0004-toaster-snapshot.patch.gz)
download
[application/x-gzip] v11-0003-toaster-default.patch.gz (34.3K, ../../CAN-LCVNLfu-1n94CnZTeD7stToJ=xXGJhrENHjqONZ3n79RRqQ@mail.gmail.com/5-v11-0003-toaster-default.patch.gz)
download
[application/x-gzip] v11-0002-toaster-interface.patch.gz (44.6K, ../../CAN-LCVNLfu-1n94CnZTeD7stToJ=xXGJhrENHjqONZ3n79RRqQ@mail.gmail.com/6-v11-0002-toaster-interface.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-08-02 15:37 Andres Freund <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Andres Freund @ 2022-08-02 15:37 UTC (permalink / raw)
To: Nikita Malakhov <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi,
On 2022-08-02 09:15:12 +0300, Nikita Malakhov wrote:
> Attach includes:
> v11-0002-toaster-interface.patch - contains TOAST API with default Toaster
> left as-is (reference implementation) and Dummy toaster as an example
> (will be removed later as a part of refactoring?).
>
> v11-0003-toaster-default.patch - implements reference TOAST as Default
> Toaster
> via TOAST API, so Heap AM calls Toast only via API, and does not have direct
> calls to Toast functionality.
>
> v11-0004-toaster-snapshot.patch - supports row versioning for TOASTed values
> and some refactoring.
I'm a bit confused by the patch numbering - why isn't there a patch 0001 and
0005?
I think 0002 needs to be split further - the relevant part isn't that it
introduces the "dummy toaster" module, it's a large change doing lots of
things, the addition of the contrib module is irrelevant comparatively.
As is the patches unfortunately are close to unreviewable. Lots of code gets
moved around in one patch, then again in the next patch, then again in the
next.
Unfortunately, scanning through these patches, it seems this is a lot of
complexity, with a (for me) comensurate benefit. There's a lot of more general
improvements to toasting and the json type that we can do, that are a lot less
complex than this.
> From 6b35d6091248e120d2361cf0a806dbfb161421cf Mon Sep 17 00:00:00 2001
> From: Nikita Malakhov <[email protected]>
> Date: Tue, 12 Apr 2022 18:37:21 +0300
> Subject: [PATCH] Pluggable TOAST API interface with dummy_toaster contrib
> module
>
> Pluggable TOAST API is introduced with implemented contrib example
> module.
> Pluggable TOAST API consists of 4 parts:
> 1) SQL syntax supports manipulations with toasters - CREATE TABLE ...
> (column type STORAGE storage_type TOASTER toaster), ALTER TABLE ALTER
> COLUMN column SET TOASTER toaster and Toaster definition.
> TOAST API requires earlier patch with CREATE TABLE SET STORAGE clause;
> New column atttoaster is added to pg_attribute.
> Toaster drop is not allowed for not to lose already toasted data;
> 2) New VARATT_CUSTOM data structure with fixed header and variable
> tail to store custom toasted data, with according macros set;
That's adding overhead to every toast interaction, independent of any new
infrastructure being used.
> 4) Dummy toaster implemented via new TOAST API to be used as sample.
> In this patch regular (default) TOAST function is left as-is and not
> yet implemented via new API.
> TOAST API syntax and code explanation provided in additional docs patch.
I'd make this a separate commit.
> @@ -445,6 +447,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
> return false;
> if (attr1->attstorage != attr2->attstorage)
> return false;
> + if (attr1->atttoaster != attr2->atttoaster)
> + return false;
So we're increasing pg_attribute - often already the largest catalog table in
a database.
Am I just missing something, or is atttoaster not actually used in this patch?
So most of the contrib module added is unreachable code?
> +/*
> + * Toasters is very often called so syscache lookup and TsrRoutine allocation are
> + * expensive and we need to cache them.
Ugh.
> + * We believe what there are only a few toasters and there is high chance that
> + * only one or only two of them are heavy used, so most used toasters should be
> + * found as easy as possible. So, let us use a simple list, in future it could
> + * be changed to other structure. For now it will be stored in TopCacheContext
> + * and never destroed in backend life cycle - toasters are never deleted.
> + */
That seems not great.
> +typedef struct ToasterCacheEntry
> +{
> + Oid toasterOid;
> + TsrRoutine *routine;
> +} ToasterCacheEntry;
> +
> +static List *ToasterCache = NIL;
> +
> +/*
> + * SearchTsrCache - get cached toaster routine, emits an error if toaster
> + * doesn't exist
> + */
> +TsrRoutine*
> +SearchTsrCache(Oid toasterOid)
> +{
> + ListCell *lc;
> + ToasterCacheEntry *entry;
> + MemoryContext ctx;
> +
> + if (list_length(ToasterCache) > 0)
> + {
> + /* fast path */
> + entry = (ToasterCacheEntry*)linitial(ToasterCache);
> + if (entry->toasterOid == toasterOid)
> + return entry->routine;
> + }
> +
> + /* didn't find in first position */
> + ctx = MemoryContextSwitchTo(CacheMemoryContext);
> +
> + for_each_from(lc, ToasterCache, 0)
> + {
> + entry = (ToasterCacheEntry*)lfirst(lc);
> +
> + if (entry->toasterOid == toasterOid)
> + {
> + /* remove entry from list, it will be added in a head of list below */
> + foreach_delete_current(ToasterCache, lc);
That needs to move later list elements!
> + goto out;
> + }
> + }
> +
> + /* did not find entry, make a new one */
> + entry = palloc(sizeof(*entry));
> +
> + entry->toasterOid = toasterOid;
> + entry->routine = GetTsrRoutineByOid(toasterOid, false);
> +
> +out:
> + ToasterCache = lcons(entry, ToasterCache);
That move the whole list around! On a cache hit. Tthis would likely already be
slower than syscache.
> diff --git a/contrib/dummy_toaster/dummy_toaster.c b/contrib/dummy_toaster/dummy_toaster.c
> index 0d261f6042..02f49052b7 100644
> --- a/contrib/dummy_toaster/dummy_toaster.c
> +++ b/contrib/dummy_toaster/dummy_toaster.c
So this is just changing around the code added in the prior commit. Why was it
then included before?
> +++ b/src/include/access/generic_toaster.h
> +HeapTuple
> +heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
> + int options);
The generic toast API has heap_* in its name?
> From 4112cd70b05dda39020d576050a98ca3cdcf2860 Mon Sep 17 00:00:00 2001
> From: Nikita Malakhov <[email protected]>
> Date: Tue, 12 Apr 2022 22:57:21 +0300
> Subject: [PATCH] Versioned rows in TOASTed values for Default Toaster support
>
> Original TOAST mechanics does not support rows versioning
> for TOASTed values.
> Toaster snapshot - refactored generic toaster, implements
> rows versions check in toasted values to share common parts
> of toasted values between different versions of rows.
This misses explaining *WHY* this is changed.
> diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
> deleted file mode 100644
> index aff8042166..0000000000
> --- a/src/backend/access/common/detoast.c
> +++ /dev/null
These patches really move things around in a largely random way.
> -static bool toastrel_valueid_exists(Relation toastrel, Oid valueid);
> -static bool toastid_valueid_exists(Oid toastrelid, Oid valueid);
> -
> +static void
> +toast_extract_chunk_fields(Relation toastrel, TupleDesc toasttupDesc,
> + Oid valueid, HeapTuple ttup, int32 *seqno,
> + char **chunkdata, int *chunksize);
> +
> +static void
> +toast_write_slice(Relation toastrel, Relation *toastidxs,
> + int num_indexes, int validIndex,
> + Oid valueid, int32 value_size, int32 slice_offset,
> + int32 slice_length, char *slice_data,
> + int options,
> + void *chunk_header, int chunk_header_size,
> + ToastChunkVisibilityCheck visibility_check,
> + void *visibility_cxt);
What do all these changes have to do with "Versioned rows in TOASTed
values for Default Toaster support"?
> +static void *
> +toast_fetch_old_chunk(Relation toastrel, SysScanDesc toastscan, Oid valueid,
> + int32 expected_chunk_seq, int32 last_old_chunk_seq,
> + ToastChunkVisibilityCheck visibility_check,
> + void *visibility_cxt,
> + int32 *p_old_chunk_size, ItemPointer old_tid)
> +{
> + for (;;)
> + {
> + HeapTuple old_toasttup;
> + char *old_chunk_data;
> + int32 old_chunk_seq;
> + int32 old_chunk_data_size;
> +
> + old_toasttup = systable_getnext_ordered(toastscan, ForwardScanDirection);
> +
> + if (old_toasttup)
> + {
> + /* Skip aborted chunks */
> + if (!HeapTupleHeaderXminCommitted(old_toasttup->t_data))
> + {
> + TransactionId xmin = HeapTupleHeaderGetXmin(old_toasttup->t_data);
> +
> + Assert(!HeapTupleHeaderXminInvalid(old_toasttup->t_data));
> +
> + if (TransactionIdDidAbort(xmin))
> + continue;
> + }
Why is there visibility logic in quite random places? Also, it's not "legal"
to call TransactionIdDidAbort() without having checked
TransactionIdIsInProgress() first. And what does this this have to do with
snapshots - it's pretty clearly not implementing snapshot logic.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-08-04 20:18 Nikita Malakhov <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Nikita Malakhov @ 2022-08-04 20:18 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Aleksander Alekseev <[email protected]>; pgsql-hackers; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
I've made a rebase according to Andres and Aleksander comments.
Rebased branch resides here:
https://github.com/postgrespro/postgres/tree/toasterapi_clean
I've decided to leave only the first 2 patches for review and send less
significant
changes after the main patch will be straightened out.
So, here is
v13-0001-toaster-interface.patch - main TOAST API patch, with reference
TOAST
mechanics left as-is.
v13-0002-toaster-default.patch - reference TOAST re-implemented via TOAST
API.
>I'm a bit confused by the patch numbering - why isn't there a patch 0001
and
>0005?
Sorry for the confusion, my fault. The first patch (CREATE TABLE .. SET
STORAGE)
is already committed into v15, and several of the late patches weren't
included.
I've rearranged patch numbers in this iteration.
>I think 0002 needs to be split further - the relevant part isn't that it
>introduces the "dummy toaster" module, it's a large change doing lots of
>things, the addition of the contrib module is irrelevant comparatively.
Done, contrib /dummy_toaster excluded from main patch and placed in branch
as a separate commit.
>As is the patches unfortunately are close to unreviewable. Lots of code
gets
>moved around in one patch, then again in the next patch, then again in the
>next.
So I've decided to put here only the first one while I'm working on the
latter to clean
this up - I agree, code in latter patches needs some refactoring. Working
on it.
>Unfortunately, scanning through these patches, it seems this is a lot of
>complexity, with a (for me) comensurate benefit. There's a lot of more
general
>improvements to toasting and the json type that we can do, that are a lot
less
>complex than this.
We have very significant improvements for storing large JSON and a couple of
other TOAST improvements which make a lot of sense, but they are based on
this API. But in the first patch reference TOAST is left as-is, and does
not use
TOAST API.
>> 2) New VARATT_CUSTOM data structure with fixed header and variable
>> tail to store custom toasted data, with according macros set;
>That's adding overhead to every toast interaction, independent of any new
>infrastructure being used.
We've performed some tests on this and haven't detected significant
overhead,
>So we're increasing pg_attribute - often already the largest catalog table
in
>a database.
A little bit, with an OID column storing Toaster OID. We do not see any
other way
to keep track of Toaster used by the table's column, because it could be
changed
any time by ALTER TABLE ... SET TOASTER.
>Am I just missing something, or is atttoaster not actually used in this
patch?
>So most of the contrib module added is unreachable code?
It is necessary for Toasters implemented via TOAST API, the first patch
does not
use it directly because reference TOAST is left unchanged. The second one
which
implements reference TOAST via TOAST API uses it.
>That seems not great.
About Toasters deletion - we forbid dropping Toasters because if Toaster is
dropped
the data TOASTed with it is lost, and as was mentioned before, we think
that there
won't be a lot of custom Toasters, likely seems to be less then a dozen.
>That move the whole list around! On a cache hit. Tthis would likely
already be
>slower than syscache.
Thank you for the remark, it is questionable approach. I've changed this in
current iteration
(patch in attach) to keep Toaster list appended-only if Toaster was not
found, and leave
Toaster cache as a straight list - first element in is the head of the list.
Also, documentation on TOAST API is provided in README.toastapi in the
first patch -
I'd be grateful for comments on it.
Thanks for the feedback!
On Tue, Aug 2, 2022 at 6:37 PM Andres Freund <[email protected]> wrote:
> Hi,
>
> On 2022-08-02 09:15:12 +0300, Nikita Malakhov wrote:
> > Attach includes:
> > v11-0002-toaster-interface.patch - contains TOAST API with default
> Toaster
> > left as-is (reference implementation) and Dummy toaster as an example
> > (will be removed later as a part of refactoring?).
> >
> > v11-0003-toaster-default.patch - implements reference TOAST as Default
> > Toaster
> > via TOAST API, so Heap AM calls Toast only via API, and does not have
> direct
> > calls to Toast functionality.
> >
> > v11-0004-toaster-snapshot.patch - supports row versioning for TOASTed
> values
> > and some refactoring.
>
> I'm a bit confused by the patch numbering - why isn't there a patch 0001
> and
> 0005?
>
> I think 0002 needs to be split further - the relevant part isn't that it
> introduces the "dummy toaster" module, it's a large change doing lots of
> things, the addition of the contrib module is irrelevant comparatively.
>
> As is the patches unfortunately are close to unreviewable. Lots of code
> gets
> moved around in one patch, then again in the next patch, then again in the
> next.
>
>
> Unfortunately, scanning through these patches, it seems this is a lot of
> complexity, with a (for me) comensurate benefit. There's a lot of more
> general
> improvements to toasting and the json type that we can do, that are a lot
> less
> complex than this.
>
>
> > From 6b35d6091248e120d2361cf0a806dbfb161421cf Mon Sep 17 00:00:00 2001
> > From: Nikita Malakhov <[email protected]>
> > Date: Tue, 12 Apr 2022 18:37:21 +0300
> > Subject: [PATCH] Pluggable TOAST API interface with dummy_toaster contrib
> > module
> >
> > Pluggable TOAST API is introduced with implemented contrib example
> > module.
> > Pluggable TOAST API consists of 4 parts:
> > 1) SQL syntax supports manipulations with toasters - CREATE TABLE ...
> > (column type STORAGE storage_type TOASTER toaster), ALTER TABLE ALTER
> > COLUMN column SET TOASTER toaster and Toaster definition.
> > TOAST API requires earlier patch with CREATE TABLE SET STORAGE clause;
> > New column atttoaster is added to pg_attribute.
> > Toaster drop is not allowed for not to lose already toasted data;
> > 2) New VARATT_CUSTOM data structure with fixed header and variable
> > tail to store custom toasted data, with according macros set;
>
> That's adding overhead to every toast interaction, independent of any new
> infrastructure being used.
>
>
>
> > 4) Dummy toaster implemented via new TOAST API to be used as sample.
> > In this patch regular (default) TOAST function is left as-is and not
> > yet implemented via new API.
> > TOAST API syntax and code explanation provided in additional docs patch.
>
> I'd make this a separate commit.
>
>
>
> > @@ -445,6 +447,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc
> tupdesc2)
> > return false;
> > if (attr1->attstorage != attr2->attstorage)
> > return false;
> > + if (attr1->atttoaster != attr2->atttoaster)
> > + return false;
>
> So we're increasing pg_attribute - often already the largest catalog table
> in
> a database.
>
> Am I just missing something, or is atttoaster not actually used in this
> patch?
> So most of the contrib module added is unreachable code?
>
>
> > +/*
> > + * Toasters is very often called so syscache lookup and TsrRoutine
> allocation are
> > + * expensive and we need to cache them.
>
> Ugh.
>
> > + * We believe what there are only a few toasters and there is high
> chance that
> > + * only one or only two of them are heavy used, so most used toasters
> should be
> > + * found as easy as possible. So, let us use a simple list, in future
> it could
> > + * be changed to other structure. For now it will be stored in
> TopCacheContext
> > + * and never destroed in backend life cycle - toasters are never
> deleted.
> > + */
>
> That seems not great.
>
>
> > +typedef struct ToasterCacheEntry
> > +{
> > + Oid toasterOid;
> > + TsrRoutine *routine;
> > +} ToasterCacheEntry;
> > +
> > +static List *ToasterCache = NIL;
> > +
> > +/*
> > + * SearchTsrCache - get cached toaster routine, emits an error if
> toaster
> > + * doesn't exist
> > + */
> > +TsrRoutine*
> > +SearchTsrCache(Oid toasterOid)
> > +{
> > + ListCell *lc;
> > + ToasterCacheEntry *entry;
> > + MemoryContext ctx;
> > +
> > + if (list_length(ToasterCache) > 0)
> > + {
> > + /* fast path */
> > + entry = (ToasterCacheEntry*)linitial(ToasterCache);
> > + if (entry->toasterOid == toasterOid)
> > + return entry->routine;
> > + }
> > +
> > + /* didn't find in first position */
> > + ctx = MemoryContextSwitchTo(CacheMemoryContext);
> > +
> > + for_each_from(lc, ToasterCache, 0)
> > + {
> > + entry = (ToasterCacheEntry*)lfirst(lc);
> > +
> > + if (entry->toasterOid == toasterOid)
> > + {
> > + /* remove entry from list, it will be added in a
> head of list below */
> > + foreach_delete_current(ToasterCache, lc);
>
> That needs to move later list elements!
>
>
> > + goto out;
> > + }
> > + }
> > +
> > + /* did not find entry, make a new one */
> > + entry = palloc(sizeof(*entry));
> > +
> > + entry->toasterOid = toasterOid;
> > + entry->routine = GetTsrRoutineByOid(toasterOid, false);
> > +
> > +out:
> > + ToasterCache = lcons(entry, ToasterCache);
>
> That move the whole list around! On a cache hit. Tthis would likely
> already be
> slower than syscache.
>
>
> > diff --git a/contrib/dummy_toaster/dummy_toaster.c
> b/contrib/dummy_toaster/dummy_toaster.c
> > index 0d261f6042..02f49052b7 100644
> > --- a/contrib/dummy_toaster/dummy_toaster.c
> > +++ b/contrib/dummy_toaster/dummy_toaster.c
>
> So this is just changing around the code added in the prior commit. Why
> was it
> then included before?
>
>
> > +++ b/src/include/access/generic_toaster.h
>
> > +HeapTuple
> > +heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple
> oldtup,
> > + int options);
>
> The generic toast API has heap_* in its name?
>
>
>
>
> > From 4112cd70b05dda39020d576050a98ca3cdcf2860 Mon Sep 17 00:00:00 2001
> > From: Nikita Malakhov <[email protected]>
> > Date: Tue, 12 Apr 2022 22:57:21 +0300
> > Subject: [PATCH] Versioned rows in TOASTed values for Default Toaster
> support
> >
> > Original TOAST mechanics does not support rows versioning
> > for TOASTed values.
> > Toaster snapshot - refactored generic toaster, implements
> > rows versions check in toasted values to share common parts
> > of toasted values between different versions of rows.
>
> This misses explaining *WHY* this is changed.
>
>
>
> > diff --git a/src/backend/access/common/detoast.c
> b/src/backend/access/common/detoast.c
> > deleted file mode 100644
> > index aff8042166..0000000000
> > --- a/src/backend/access/common/detoast.c
> > +++ /dev/null
>
> These patches really move things around in a largely random way.
>
>
> > -static bool toastrel_valueid_exists(Relation toastrel, Oid valueid);
> > -static bool toastid_valueid_exists(Oid toastrelid, Oid valueid);
> > -
> > +static void
> > +toast_extract_chunk_fields(Relation toastrel, TupleDesc toasttupDesc,
> > + Oid valueid, HeapTuple
> ttup, int32 *seqno,
> > + char **chunkdata, int
> *chunksize);
> > +
> > +static void
> > +toast_write_slice(Relation toastrel, Relation *toastidxs,
> > + int num_indexes, int validIndex,
> > + Oid valueid, int32 value_size, int32
> slice_offset,
> > + int32 slice_length, char *slice_data,
> > + int options,
> > + void *chunk_header, int
> chunk_header_size,
> > + ToastChunkVisibilityCheck
> visibility_check,
> > + void *visibility_cxt);
>
> What do all these changes have to do with "Versioned rows in TOASTed
> values for Default Toaster support"?
>
>
> > +static void *
> > +toast_fetch_old_chunk(Relation toastrel, SysScanDesc toastscan, Oid
> valueid,
> > + int32 expected_chunk_seq, int32
> last_old_chunk_seq,
> > + ToastChunkVisibilityCheck
> visibility_check,
> > + void *visibility_cxt,
> > + int32 *p_old_chunk_size,
> ItemPointer old_tid)
> > +{
> > + for (;;)
> > + {
> > + HeapTuple old_toasttup;
> > + char *old_chunk_data;
> > + int32 old_chunk_seq;
> > + int32 old_chunk_data_size;
> > +
> > + old_toasttup = systable_getnext_ordered(toastscan,
> ForwardScanDirection);
> > +
> > + if (old_toasttup)
> > + {
> > + /* Skip aborted chunks */
> > + if
> (!HeapTupleHeaderXminCommitted(old_toasttup->t_data))
> > + {
> > + TransactionId xmin =
> HeapTupleHeaderGetXmin(old_toasttup->t_data);
> > +
> > +
> Assert(!HeapTupleHeaderXminInvalid(old_toasttup->t_data));
> > +
> > + if (TransactionIdDidAbort(xmin))
> > + continue;
> > + }
>
> Why is there visibility logic in quite random places? Also, it's not
> "legal"
> to call TransactionIdDidAbort() without having checked
> TransactionIdIsInProgress() first. And what does this this have to do with
> snapshots - it's pretty clearly not implementing snapshot logic.
>
>
> Greetings,
>
> Andres Freund
>
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Attachments:
[application/x-gzip] v13-0002-toaster-default.patch.gz (29.3K, ../../CAN-LCVMXN-igZdOH4kunF0p2jobsksNhFXovEXjr-zVF90Nq1A@mail.gmail.com/3-v13-0002-toaster-default.patch.gz)
download
[application/x-gzip] v13-0001-toaster-interface.patch.gz (48.8K, ../../CAN-LCVMXN-igZdOH4kunF0p2jobsksNhFXovEXjr-zVF90Nq1A@mail.gmail.com/4-v13-0001-toaster-interface.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-08-23 08:27 Aleksander Alekseev <[email protected]>
parent: Nikita Malakhov <[email protected]>
0 siblings, 1 reply; 39+ messages in thread
From: Aleksander Alekseev @ 2022-08-23 08:27 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Nikita Malakhov <[email protected]>; Andres Freund <[email protected]>; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi Nikita,
> I've decided to leave only the first 2 patches for review and send less significant
> changes after the main patch will be straightened out.
> So, here is
> v13-0001-toaster-interface.patch - main TOAST API patch, with reference TOAST
> mechanics left as-is.
> v13-0002-toaster-default.patch - reference TOAST re-implemented via TOAST API.
> [...]
Great! Thank you.
Unfortunately the patchset still seems to have difficulties passing
the CI checks (see http://cfbot.cputube.org/ ). Any chance we may see
a version rebased to the current `master` branch for the September CF?
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 39+ messages in thread
* Re: Pluggable toaster
@ 2022-08-24 09:59 Nikita Malakhov <[email protected]>
parent: Aleksander Alekseev <[email protected]>
0 siblings, 0 replies; 39+ messages in thread
From: Nikita Malakhov @ 2022-08-24 09:59 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: pgsql-hackers; Andres Freund <[email protected]>; Matthias van de Meent <[email protected]>; Robert Haas <[email protected]>; Greg Stark <[email protected]>; Teodor Sigaev <[email protected]>
Hi hackers!
I've rebased actual branch onto the latest master and re-created patches.
Checked with git am,
all applied correctly. Please check the attached patches.
Rebased branch resides here:
https://github.com/postgrespro/postgres/tree/toasterapi_clean
Just to remind - I've decided to leave only the first 2 patches for review
and send less significant
changes after the main patch will be straightened out.
So, here is
v14-0001-toaster-interface.patch - main TOAST API patch, with reference
TOAST
mechanics left as-is.
v14-0002-toaster-default.patch - reference TOAST re-implemented via TOAST
API.
On Tue, Aug 23, 2022 at 11:27 AM Aleksander Alekseev <
[email protected]> wrote:
> Hi Nikita,
>
> > I've decided to leave only the first 2 patches for review and send less
> significant
> > changes after the main patch will be straightened out.
> > So, here is
> > v13-0001-toaster-interface.patch - main TOAST API patch, with reference
> TOAST
> > mechanics left as-is.
> > v13-0002-toaster-default.patch - reference TOAST re-implemented via
> TOAST API.
> > [...]
>
> Great! Thank you.
>
> Unfortunately the patchset still seems to have difficulties passing
> the CI checks (see http://cfbot.cputube.org/ ). Any chance we may see
> a version rebased to the current `master` branch for the September CF?
>
> --
> Best regards,
> Aleksander Alekseev
>
--
Regards,
Nikita Malakhov
https://postgrespro.ru/
Attachments:
[application/x-gzip] v14-0002-toaster-default.patch.gz (26.2K, ../../CAN-LCVMpb5t0QheofS=9gQbKSY-Hf_ESCRjBeG-pbL90gRprag@mail.gmail.com/3-v14-0002-toaster-default.patch.gz)
download
[application/x-gzip] v14-0001-toaster-interface.patch.gz (48.8K, ../../CAN-LCVMpb5t0QheofS=9gQbKSY-Hf_ESCRjBeG-pbL90gRprag@mail.gmail.com/4-v14-0001-toaster-interface.patch.gz)
download
^ permalink raw reply [nested|flat] 39+ messages in thread
end of thread, other threads:[~2022-08-24 09:59 UTC | newest]
Thread overview: 39+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-02-01 15:08 [PATCH v3] Move page initialization from RelationAddExtraBlocks() to use, take 2. Andres Freund <[email protected]>
2022-02-02 07:34 Re: Pluggable toaster Teodor Sigaev <[email protected]>
2022-03-10 08:47 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-03-21 23:31 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-03-22 00:51 ` Re: Pluggable toaster Andres Freund <[email protected]>
2022-03-22 12:18 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-03-31 20:14 ` Re: Pluggable toaster Greg Stark <[email protected]>
2022-04-01 14:11 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-04-03 01:20 ` Re: Pluggable toaster Greg Stark <[email protected]>
2022-04-03 02:06 ` Re: Pluggable toaster Andres Freund <[email protected]>
2022-04-03 13:15 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-04-04 16:12 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-04-04 16:39 ` Re: Pluggable toaster Robert Haas <[email protected]>
2022-04-04 19:32 ` Re: Pluggable toaster Greg Stark <[email protected]>
2022-04-04 20:05 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-04-04 20:17 ` Re: Pluggable toaster Robert Haas <[email protected]>
2022-04-05 07:58 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-04-13 18:55 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-04-13 19:58 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-06-17 14:33 ` Re: Pluggable toaster Aleksander Alekseev <[email protected]>
2022-06-23 11:46 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-06-23 13:38 ` Re: Pluggable toaster Aleksander Alekseev <[email protected]>
2022-06-23 13:53 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-06-30 20:26 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-06-30 20:27 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-07-01 12:27 ` Re: Pluggable toaster Matthias van de Meent <[email protected]>
2022-07-11 12:03 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-07-13 19:45 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-07-20 09:15 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-07-22 13:16 ` Re: Pluggable toaster Matthias van de Meent <[email protected]>
2022-07-23 07:15 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-07-25 21:20 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-07-26 08:22 ` Re: Pluggable toaster Aleksander Alekseev <[email protected]>
2022-07-29 06:16 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-08-02 06:15 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-08-02 15:37 ` Re: Pluggable toaster Andres Freund <[email protected]>
2022-08-04 20:18 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
2022-08-23 08:27 ` Re: Pluggable toaster Aleksander Alekseev <[email protected]>
2022-08-24 09:59 ` Re: Pluggable toaster Nikita Malakhov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox