ALTER TABLE uses a bistate but not for toast tables
Started by Justin Pryzbyover 3 years ago11 messages
ATRewriteTable() calls table_tuple_insert() with a bistate, to avoid clobbering
and polluting the buffers.
But heap_insert() then calls
heap_prepare_insert() >
heap_toast_insert_or_update >
toast_tuple_externalize >
toast_save_datum >
heap_insert(toastrel, toasttup, mycid, options, NULL /* without bistate:( */);
I came up with this patch. I'm not sure but maybe it should be implemented at
the tableam layer and not inside heap. Maybe the BulkInsertState should have a
2nd strategy buffer for toast tables.
CREATE TABLE t(i int, a text, b text, c text,d text,e text,f text,g text);
INSERT INTO t SELECT 0, array_agg(a),array_agg(a),array_agg(a),array_agg(a),array_agg(a),array_agg(a) FROM generate_series(1,999)n,repeat(n::text,99)a,generate_series(1,99)b GROUP BY b;
INSERT INTO t SELECT * FROM t;
INSERT INTO t SELECT * FROM t;
INSERT INTO t SELECT * FROM t;
INSERT INTO t SELECT * FROM t;
ALTER TABLE t ALTER i TYPE smallint;
SELECT COUNT(1), relname, COUNT(1) FILTER(WHERE isdirty) FROM pg_buffercache b JOIN pg_class c ON c.oid=b.relfilenode GROUP BY 2 ORDER BY 1 DESC LIMIT 9;
Without this patch:
postgres=# SELECT COUNT(1), relname, COUNT(1) FILTER(WHERE isdirty) FROM pg_buffercache b JOIN pg_class c ON c.oid=b.relfilenode GROUP BY 2 ORDER BY 1 DESC LIMIT 9;
10283 | pg_toast_55759 | 8967
With this patch:
1418 | pg_toast_16597 | 1418
--
Justin
Attachments:
0001-WIP-use-BulkInsertState-for-toast-tuples-too.patchtext/x-diff; charset=us-asciiDownload
From 2b156036856dcbeab00de819e5c6eff820b564cd Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 21 Jun 2022 22:28:06 -0500
Subject: [PATCH] WIP: use BulkInsertState for toast tuples, too
ci-os-only: linux
---
src/backend/access/common/toast_internals.c | 16 ++++++++++++++--
src/backend/access/heap/heapam.c | 14 ++++++++------
src/backend/access/heap/heaptoast.c | 11 +++++++----
src/backend/access/heap/rewriteheap.c | 2 +-
src/backend/access/table/toast_helper.c | 6 ++++--
src/include/access/heaptoast.h | 4 +++-
src/include/access/toast_helper.h | 3 ++-
src/include/access/toast_internals.h | 4 +++-
8 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 576e585a89f..2640e6a1589 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -118,7 +118,8 @@ toast_compress_datum(Datum value, char cmethod)
*/
Datum
toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options)
+ struct varlena *oldexternal, int options,
+ BulkInsertState bistate)
{
Relation toastrel;
Relation *toastidxs;
@@ -299,6 +300,10 @@ toast_save_datum(Relation rel, Datum value,
t_isnull[1] = false;
t_isnull[2] = false;
+ /* Release pin after main table, before switching to write to toast table */
+ if (bistate)
+ ReleaseBulkInsertStatePin(bistate);
+
/*
* Split up the item into chunks
*/
@@ -321,7 +326,7 @@ toast_save_datum(Relation rel, Datum value,
memcpy(VARDATA(&chunk_data), data_p, chunk_size);
toasttup = heap_form_tuple(toasttupDesc, t_values, t_isnull);
- heap_insert(toastrel, toasttup, mycid, options, NULL);
+ heap_insert(toastrel, toasttup, mycid, options, bistate);
/*
* Create the index entry. We cheat a little here by not using
@@ -358,6 +363,13 @@ toast_save_datum(Relation rel, Datum value,
data_p += chunk_size;
}
+ if (bistate)
+ {
+ table_finish_bulk_insert(toastrel, options); // XXX
+ /* Release pin after writing toast table before resuming writes to the main table */
+ ReleaseBulkInsertStatePin(bistate);
+ }
+
/*
* Done - close toast relation and its indexes but keep the lock until
* commit, so as a concurrent reindex done directly on the toast relation
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 74218510276..e46a6035068 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -73,7 +73,8 @@
static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
- TransactionId xid, CommandId cid, int options);
+ TransactionId xid, CommandId cid, int options,
+ BulkInsertState bistate);
static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tuple,
@@ -2042,7 +2043,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* Note: below this point, heaptup is the data we actually intend to store
* into the relation; tup is the caller's original untoasted data.
*/
- heaptup = heap_prepare_insert(relation, tup, xid, cid, options);
+
+ heaptup = heap_prepare_insert(relation, tup, xid, cid, options, bistate);
/*
* Find buffer to insert this tuple into. If the page is all visible,
@@ -2212,7 +2214,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
*/
static HeapTuple
heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
- CommandId cid, int options)
+ CommandId cid, int options, BulkInsertState bistate)
{
/*
* To allow parallel inserts, we need to ensure that they are safe to be
@@ -2248,7 +2250,7 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
return tup;
}
else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
- return heap_toast_insert_or_update(relation, tup, NULL, options);
+ return heap_toast_insert_or_update(relation, tup, NULL, options, bistate);
else
return tup;
}
@@ -2297,7 +2299,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
slots[i]->tts_tableOid = RelationGetRelid(relation);
tuple->t_tableOid = slots[i]->tts_tableOid;
heaptuples[i] = heap_prepare_insert(relation, tuple, xid, cid,
- options);
+ options, NULL);
}
/*
@@ -3724,7 +3726,7 @@ l2:
if (need_toast)
{
/* Note we always use WAL and FSM during updates */
- heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0);
+ heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0, NULL);
newtupsize = MAXALIGN(heaptup->t_len);
}
else
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index 1575a81b01b..25b07f82cb6 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -94,7 +94,7 @@ heap_toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative)
*/
HeapTuple
heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
- int options)
+ int options, BulkInsertState bistate)
{
HeapTuple result_tuple;
TupleDesc tupleDesc;
@@ -110,6 +110,9 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
ToastAttrInfo toast_attr[MaxHeapAttributeNumber];
ToastTupleContext ttc;
+ /* Bulk insert is not supported for updates, only inserts. */
+ Assert(bistate == NULL || oldtup == NULL);
+
/*
* Ignore the INSERT_SPECULATIVE option. Speculative insertions/super
* deletions just normally insert/delete the toast values. It seems
@@ -214,7 +217,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
*/
if (toast_attr[biggest_attno].tai_size > maxDataLen &&
rel->rd_rel->reltoastrelid != InvalidOid)
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -231,7 +234,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
biggest_attno = toast_tuple_find_biggest_attribute(&ttc, false, false);
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -267,7 +270,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 2a53826736e..caff247472f 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -644,7 +644,7 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
options |= HEAP_INSERT_NO_LOGICAL;
heaptup = heap_toast_insert_or_update(state->rs_new_rel, tup, NULL,
- options);
+ options, NULL); // XXX
}
else
heaptup = tup;
diff --git a/src/backend/access/table/toast_helper.c b/src/backend/access/table/toast_helper.c
index 0cc5a30f9b4..764c7a265d5 100644
--- a/src/backend/access/table/toast_helper.c
+++ b/src/backend/access/table/toast_helper.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include "access/detoast.h"
+#include "access/hio.h"
#include "access/table.h"
#include "access/toast_helper.h"
#include "access/toast_internals.h"
@@ -253,7 +254,8 @@ toast_tuple_try_compression(ToastTupleContext *ttc, int attribute)
* Move an attribute to external storage.
*/
void
-toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
+toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options,
+ BulkInsertStateData *bistate)
{
Datum *value = &ttc->ttc_values[attribute];
Datum old_value = *value;
@@ -261,7 +263,7 @@ toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
attr->tai_colflags |= TOASTCOL_IGNORE;
*value = toast_save_datum(ttc->ttc_rel, old_value, attr->tai_oldexternal,
- options);
+ options, bistate);
if ((attr->tai_colflags & TOASTCOL_NEEDS_FREE) != 0)
pfree(DatumGetPointer(old_value));
attr->tai_colflags |= TOASTCOL_NEEDS_FREE;
diff --git a/src/include/access/heaptoast.h b/src/include/access/heaptoast.h
index a75699054af..2db01894c52 100644
--- a/src/include/access/heaptoast.h
+++ b/src/include/access/heaptoast.h
@@ -13,6 +13,7 @@
#ifndef HEAPTOAST_H
#define HEAPTOAST_H
+#include "access/hio.h"
#include "access/htup_details.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -95,7 +96,8 @@
* ----------
*/
extern HeapTuple heap_toast_insert_or_update(Relation rel, HeapTuple newtup,
- HeapTuple oldtup, int options);
+ HeapTuple oldtup, int options,
+ BulkInsertStateData *bistate);
/* ----------
* heap_toast_delete -
diff --git a/src/include/access/toast_helper.h b/src/include/access/toast_helper.h
index 1e2aaf3303e..46c358b1c29 100644
--- a/src/include/access/toast_helper.h
+++ b/src/include/access/toast_helper.h
@@ -14,6 +14,7 @@
#ifndef TOAST_HELPER_H
#define TOAST_HELPER_H
+#include "access/hio.h"
#include "utils/rel.h"
/*
@@ -107,7 +108,7 @@ extern int toast_tuple_find_biggest_attribute(ToastTupleContext *ttc,
bool check_main);
extern void toast_tuple_try_compression(ToastTupleContext *ttc, int attribute);
extern void toast_tuple_externalize(ToastTupleContext *ttc, int attribute,
- int options);
+ int options, BulkInsertStateData *bistate);
extern void toast_tuple_cleanup(ToastTupleContext *ttc);
extern void toast_delete_external(Relation rel, Datum *values, bool *isnull,
diff --git a/src/include/access/toast_internals.h b/src/include/access/toast_internals.h
index 85e7dc0fc5f..a9265a1856b 100644
--- a/src/include/access/toast_internals.h
+++ b/src/include/access/toast_internals.h
@@ -12,6 +12,7 @@
#ifndef TOAST_INTERNALS_H
#define TOAST_INTERNALS_H
+#include "access/hio.h"
#include "access/toast_compression.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -50,7 +51,8 @@ extern Oid toast_get_valid_index(Oid toastoid, LOCKMODE lock);
extern void toast_delete_datum(Relation rel, Datum value, bool is_speculative);
extern Datum toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options);
+ struct varlena *oldexternal, int options,
+ BulkInsertStateData *bistate);
extern int toast_open_indexes(Relation toastrel,
LOCKMODE lock,
--
2.17.1
Re: ALTER TABLE uses a bistate but not for toast tables
Hi,
On 6/22/22 4:38 PM, Justin Pryzby wrote:
ATRewriteTable() calls table_tuple_insert() with a bistate, to avoid clobbering
and polluting the buffers.But heap_insert() then calls
heap_prepare_insert() >
heap_toast_insert_or_update >
toast_tuple_externalize >
toast_save_datum >
heap_insert(toastrel, toasttup, mycid, options, NULL /* without bistate:( */);
Good catch!
I came up with this patch.
+ /* Release pin after main table, before switching to write to
toast table */
+ if (bistate)
+ ReleaseBulkInsertStatePin(bistate);I'm not sure we should release and reuse here the bistate of the main
table: it looks like that with the patch ReadBufferBI() on the main
relation wont have the desired block already pinned (then would need to
perform a read).
What do you think about creating earlier a new dedicated bistate for the
toast table?
+ if (bistate)
+ {
+ table_finish_bulk_insert(toastrel, options); // XXXI think it's too early, as it looks to me that at this stage we may have
not finished the whole bulk insert yet.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services:https://aws.amazon.com
Re: ALTER TABLE uses a bistate but not for toast tables
On Wed, Sep 07, 2022 at 10:48:39AM +0200, Drouvot, Bertrand wrote:
+ if (bistate) + { + table_finish_bulk_insert(toastrel, options); // XXXI think it's too early, as it looks to me that at this stage we may have not
finished the whole bulk insert yet.
Yeah, that feels fishy. Not sure what's the idea behind the XXX
comment, either. I have marked this patch as RwF, following the lack
of reply.
--
Michael
Re: ALTER TABLE uses a bistate but not for toast tables
On Wed, Sep 07, 2022 at 10:48:39AM +0200, Drouvot, Bertrand wrote:
Hi,
On 6/22/22 4:38 PM, Justin Pryzby wrote:
ATRewriteTable() calls table_tuple_insert() with a bistate, to avoid clobbering
and polluting the buffers.But heap_insert() then calls
heap_prepare_insert() >
heap_toast_insert_or_update >
toast_tuple_externalize >
toast_save_datum >
heap_insert(toastrel, toasttup, mycid, options, NULL /* without bistate:( */);What do you think about creating earlier a new dedicated bistate for the
toast table?
Yes, but I needed to think about what data structure to put it in...
Here, I created a 2nd bistate for toast whenever creating a bistate for
heap. That avoids the need to add arguments to tableam's
table_tuple_insert(), in addition to the 6 other functions in the call
stack.
I also updated rewriteheap.c to handle the same problem in CLUSTER:
postgres=# DROP TABLE t; CREATE TABLE t AS SELECT i, repeat((5555555+i)::text, 123456)t FROM generate_series(1,9999)i;
postgres=# VACUUM FULL VERBOSE t ; SELECT COUNT(1), datname, coalesce(c.relname,b.relfilenode::text), d.relname FROM pg_buffercache b LEFT JOIN pg_class c ON b.relfilenode=pg_relation_filenode(c.oid) LEFT JOIN pg_class d ON d.reltoastrelid=c.oid LEFT JOIN pg_database db ON db.oid=b.reldatabase GROUP BY 2,3,4 ORDER BY 1 DESC LIMIT 22;
Unpatched:
5000 | postgres | pg_toast_96188840 | t
=> 40MB of shared buffers
Patched:
2048 | postgres | pg_toast_17097 | t
Note that a similar problem seems to exist in COPY ... but I can't see
how to fix that one.
--
Justin
Attachments:
v3-0001-WIP-use-BulkInsertState-for-toast-tuples-too.patchtext/x-diff; charset=us-asciiDownload
From 90449459a6d8c4f87bfe23f462a45649beed4a8d Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 21 Jun 2022 22:28:06 -0500
Subject: [PATCH v3] WIP: use BulkInsertState for toast tuples, too
DONE: ALTER, CLUSTER
TODO: copyto, copyfrom?
slot_getsomeattrs slot_deform_heap_tuple fetchatt
heap_getnextslot => heapgettup => heapgetpage => ReadBufferExtended
initscan
table_beginscan table_scan_getnextslot
RelationCopyStorageUsingBuffer ReadBufferWithoutRelcache
(gdb) bt
#0 table_open (relationId=relationId@entry=16390, lockmode=lockmode@entry=1) at table.c:40
#1 0x000056444cb23d3c in toast_fetch_datum (attr=attr@entry=0x7f67933cc6cc) at detoast.c:372
#2 0x000056444cb24217 in detoast_attr (attr=attr@entry=0x7f67933cc6cc) at detoast.c:123
#3 0x000056444d07a4c8 in pg_detoast_datum_packed (datum=datum@entry=0x7f67933cc6cc) at fmgr.c:1743
#4 0x000056444d042c8d in text_to_cstring (t=0x7f67933cc6cc) at varlena.c:224
#5 0x000056444d0434f9 in textout (fcinfo=<optimized out>) at varlena.c:573
#6 0x000056444d078f10 in FunctionCall1Coll (flinfo=flinfo@entry=0x56444e4706b0, collation=collation@entry=0, arg1=arg1@entry=140082828592844) at fmgr.c:1124
#7 0x000056444d079d7f in OutputFunctionCall (flinfo=flinfo@entry=0x56444e4706b0, val=val@entry=140082828592844) at fmgr.c:1561
#8 0x000056444ccb1665 in CopyOneRowTo (cstate=cstate@entry=0x56444e470898, slot=slot@entry=0x56444e396d20) at copyto.c:975
#9 0x000056444ccb2c7d in DoCopyTo (cstate=cstate@entry=0x56444e470898) at copyto.c:891
#10 0x000056444ccab4c2 in DoCopy (pstate=pstate@entry=0x56444e396bb0, stmt=stmt@entry=0x56444e3759b0, stmt_location=0, stmt_len=48, processed=processed@entry=0x7ffc212a6310) at copy.c:308
cluster:
heapam_relation_copy_for_cluster
reform_and_rewrite_tuple
rewrite_heap_tuple
raw_heap_insert
//-os-only: linux
---
src/backend/access/common/toast_internals.c | 6 ++++--
src/backend/access/heap/heapam.c | 23 +++++++++++++++------
src/backend/access/heap/heaptoast.c | 11 ++++++----
src/backend/access/heap/rewriteheap.c | 6 +++++-
src/backend/access/table/toast_helper.c | 6 ++++--
src/include/access/heaptoast.h | 4 +++-
src/include/access/hio.h | 1 +
src/include/access/toast_helper.h | 3 ++-
src/include/access/toast_internals.h | 4 +++-
9 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 576e585a89f..e5f8a0b7073 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -118,7 +118,8 @@ toast_compress_datum(Datum value, char cmethod)
*/
Datum
toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options)
+ struct varlena *oldexternal, int options,
+ BulkInsertState bistate)
{
Relation toastrel;
Relation *toastidxs;
@@ -321,7 +322,8 @@ toast_save_datum(Relation rel, Datum value,
memcpy(VARDATA(&chunk_data), data_p, chunk_size);
toasttup = heap_form_tuple(toasttupDesc, t_values, t_isnull);
- heap_insert(toastrel, toasttup, mycid, options, NULL);
+ heap_insert(toastrel, toasttup, mycid, options, bistate ?
+ bistate->toast_state : NULL);
/*
* Create the index entry. We cheat a little here by not using
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 747db503761..69539429e96 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -73,7 +73,8 @@
static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
- TransactionId xid, CommandId cid, int options);
+ TransactionId xid, CommandId cid, int options,
+ BulkInsertState bistate);
static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tuple,
@@ -1980,6 +1981,11 @@ GetBulkInsertState(void)
bistate = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
bistate->strategy = GetAccessStrategy(BAS_BULKWRITE);
bistate->current_buf = InvalidBuffer;
+
+ bistate->toast_state = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
+ bistate->toast_state->strategy = GetAccessStrategy(BAS_BULKWRITE);
+ bistate->toast_state->current_buf = InvalidBuffer;
+
return bistate;
}
@@ -1991,6 +1997,10 @@ FreeBulkInsertState(BulkInsertState bistate)
{
if (bistate->current_buf != InvalidBuffer)
ReleaseBuffer(bistate->current_buf);
+ if (bistate->toast_state->current_buf != InvalidBuffer)
+ ReleaseBuffer(bistate->toast_state->current_buf);
+
+ FreeAccessStrategy(bistate->toast_state->strategy);
FreeAccessStrategy(bistate->strategy);
pfree(bistate);
}
@@ -2045,7 +2055,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* Note: below this point, heaptup is the data we actually intend to store
* into the relation; tup is the caller's original untoasted data.
*/
- heaptup = heap_prepare_insert(relation, tup, xid, cid, options);
+
+ heaptup = heap_prepare_insert(relation, tup, xid, cid, options, bistate);
/*
* Find buffer to insert this tuple into. If the page is all visible,
@@ -2215,7 +2226,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
*/
static HeapTuple
heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
- CommandId cid, int options)
+ CommandId cid, int options, BulkInsertState bistate)
{
/*
* To allow parallel inserts, we need to ensure that they are safe to be
@@ -2251,7 +2262,7 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
return tup;
}
else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
- return heap_toast_insert_or_update(relation, tup, NULL, options);
+ return heap_toast_insert_or_update(relation, tup, NULL, options, bistate);
else
return tup;
}
@@ -2300,7 +2311,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
slots[i]->tts_tableOid = RelationGetRelid(relation);
tuple->t_tableOid = slots[i]->tts_tableOid;
heaptuples[i] = heap_prepare_insert(relation, tuple, xid, cid,
- options);
+ options, NULL);
}
/*
@@ -3735,7 +3746,7 @@ l2:
if (need_toast)
{
/* Note we always use WAL and FSM during updates */
- heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0);
+ heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0, NULL);
newtupsize = MAXALIGN(heaptup->t_len);
}
else
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index 1575a81b01b..25b07f82cb6 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -94,7 +94,7 @@ heap_toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative)
*/
HeapTuple
heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
- int options)
+ int options, BulkInsertState bistate)
{
HeapTuple result_tuple;
TupleDesc tupleDesc;
@@ -110,6 +110,9 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
ToastAttrInfo toast_attr[MaxHeapAttributeNumber];
ToastTupleContext ttc;
+ /* Bulk insert is not supported for updates, only inserts. */
+ Assert(bistate == NULL || oldtup == NULL);
+
/*
* Ignore the INSERT_SPECULATIVE option. Speculative insertions/super
* deletions just normally insert/delete the toast values. It seems
@@ -214,7 +217,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
*/
if (toast_attr[biggest_attno].tai_size > maxDataLen &&
rel->rd_rel->reltoastrelid != InvalidOid)
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -231,7 +234,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
biggest_attno = toast_tuple_find_biggest_attribute(&ttc, false, false);
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -267,7 +270,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 2fe9e48e500..b18f4e763f5 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -152,6 +152,7 @@ typedef struct RewriteStateData
HTAB *rs_old_new_tid_map; /* unmatched B tuples */
HTAB *rs_logical_mappings; /* logical remapping files */
uint32 rs_num_rewrite_mappings; /* # in memory mappings */
+ BulkInsertState rs_bistate;
} RewriteStateData;
/*
@@ -263,6 +264,7 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
state->rs_freeze_xid = freeze_xid;
state->rs_cutoff_multi = cutoff_multi;
state->rs_cxt = rw_cxt;
+ state->rs_bistate = GetBulkInsertState();
/* Initialize hash tables used to track update chains */
hash_ctl.keysize = sizeof(TidHashKey);
@@ -313,6 +315,8 @@ end_heap_rewrite(RewriteState state)
raw_heap_insert(state, unresolved->tuple);
}
+ FreeBulkInsertState(state->rs_bistate);
+
/* Write the last page, if any */
if (state->rs_buffer_valid)
{
@@ -643,7 +647,7 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
options |= HEAP_INSERT_NO_LOGICAL;
heaptup = heap_toast_insert_or_update(state->rs_new_rel, tup, NULL,
- options);
+ options, state->rs_bistate);
}
else
heaptup = tup;
diff --git a/src/backend/access/table/toast_helper.c b/src/backend/access/table/toast_helper.c
index 74ba2189f0a..7cf56866609 100644
--- a/src/backend/access/table/toast_helper.c
+++ b/src/backend/access/table/toast_helper.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include "access/detoast.h"
+#include "access/hio.h"
#include "access/table.h"
#include "access/toast_helper.h"
#include "access/toast_internals.h"
@@ -253,7 +254,8 @@ toast_tuple_try_compression(ToastTupleContext *ttc, int attribute)
* Move an attribute to external storage.
*/
void
-toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
+toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options,
+ BulkInsertStateData *bistate)
{
Datum *value = &ttc->ttc_values[attribute];
Datum old_value = *value;
@@ -261,7 +263,7 @@ toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
attr->tai_colflags |= TOASTCOL_IGNORE;
*value = toast_save_datum(ttc->ttc_rel, old_value, attr->tai_oldexternal,
- options);
+ options, bistate);
if ((attr->tai_colflags & TOASTCOL_NEEDS_FREE) != 0)
pfree(DatumGetPointer(old_value));
attr->tai_colflags |= TOASTCOL_NEEDS_FREE;
diff --git a/src/include/access/heaptoast.h b/src/include/access/heaptoast.h
index a75699054af..2db01894c52 100644
--- a/src/include/access/heaptoast.h
+++ b/src/include/access/heaptoast.h
@@ -13,6 +13,7 @@
#ifndef HEAPTOAST_H
#define HEAPTOAST_H
+#include "access/hio.h"
#include "access/htup_details.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -95,7 +96,8 @@
* ----------
*/
extern HeapTuple heap_toast_insert_or_update(Relation rel, HeapTuple newtup,
- HeapTuple oldtup, int options);
+ HeapTuple oldtup, int options,
+ BulkInsertStateData *bistate);
/* ----------
* heap_toast_delete -
diff --git a/src/include/access/hio.h b/src/include/access/hio.h
index bb90c6fad81..12668091ffa 100644
--- a/src/include/access/hio.h
+++ b/src/include/access/hio.h
@@ -30,6 +30,7 @@ typedef struct BulkInsertStateData
{
BufferAccessStrategy strategy; /* our BULKWRITE strategy object */
Buffer current_buf; /* current insertion target page */
+ struct BulkInsertStateData *toast_state;
} BulkInsertStateData;
diff --git a/src/include/access/toast_helper.h b/src/include/access/toast_helper.h
index 1e2aaf3303e..46c358b1c29 100644
--- a/src/include/access/toast_helper.h
+++ b/src/include/access/toast_helper.h
@@ -14,6 +14,7 @@
#ifndef TOAST_HELPER_H
#define TOAST_HELPER_H
+#include "access/hio.h"
#include "utils/rel.h"
/*
@@ -107,7 +108,7 @@ extern int toast_tuple_find_biggest_attribute(ToastTupleContext *ttc,
bool check_main);
extern void toast_tuple_try_compression(ToastTupleContext *ttc, int attribute);
extern void toast_tuple_externalize(ToastTupleContext *ttc, int attribute,
- int options);
+ int options, BulkInsertStateData *bistate);
extern void toast_tuple_cleanup(ToastTupleContext *ttc);
extern void toast_delete_external(Relation rel, Datum *values, bool *isnull,
diff --git a/src/include/access/toast_internals.h b/src/include/access/toast_internals.h
index 85e7dc0fc5f..a9265a1856b 100644
--- a/src/include/access/toast_internals.h
+++ b/src/include/access/toast_internals.h
@@ -12,6 +12,7 @@
#ifndef TOAST_INTERNALS_H
#define TOAST_INTERNALS_H
+#include "access/hio.h"
#include "access/toast_compression.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -50,7 +51,8 @@ extern Oid toast_get_valid_index(Oid toastoid, LOCKMODE lock);
extern void toast_delete_datum(Relation rel, Datum value, bool is_speculative);
extern Datum toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options);
+ struct varlena *oldexternal, int options,
+ BulkInsertStateData *bistate);
extern int toast_open_indexes(Relation toastrel,
LOCKMODE lock,
--
2.25.1
Re: ALTER TABLE uses a bistate but not for toast tables
Hi!
Found this discussion for our experiments with TOAST, I'd have to check it
under [1]/messages/by-id/224711f9-83b7-a307-b17f-4457ab73aa0a@sigaev.ru.
I'm not sure, what behavior is expected when the main table is unpinned,
bulk insert
to the TOAST table is in progress, and the second query with a heavy bulk
insert to
the same TOAST table comes in?
Thank you!
[1]: /messages/by-id/224711f9-83b7-a307-b17f-4457ab73aa0a@sigaev.ru
/messages/by-id/224711f9-83b7-a307-b17f-4457ab73aa0a@sigaev.ru
On Sun, Nov 27, 2022 at 11:15 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
On Wed, Sep 07, 2022 at 10:48:39AM +0200, Drouvot, Bertrand wrote:
Hi,
On 6/22/22 4:38 PM, Justin Pryzby wrote:
ATRewriteTable() calls table_tuple_insert() with a bistate, to avoid
clobbering
and polluting the buffers.
But heap_insert() then calls
heap_prepare_insert() >
heap_toast_insert_or_update >
toast_tuple_externalize >
toast_save_datum >
heap_insert(toastrel, toasttup, mycid, options, NULL /* withoutbistate:( */);
What do you think about creating earlier a new dedicated bistate for the
toast table?Yes, but I needed to think about what data structure to put it in...
Here, I created a 2nd bistate for toast whenever creating a bistate for
heap. That avoids the need to add arguments to tableam's
table_tuple_insert(), in addition to the 6 other functions in the call
stack.I also updated rewriteheap.c to handle the same problem in CLUSTER:
postgres=# DROP TABLE t; CREATE TABLE t AS SELECT i,
repeat((5555555+i)::text, 123456)t FROM generate_series(1,9999)i;
postgres=# VACUUM FULL VERBOSE t ; SELECT COUNT(1), datname,
coalesce(c.relname,b.relfilenode::text), d.relname FROM pg_buffercache b
LEFT JOIN pg_class c ON b.relfilenode=pg_relation_filenode(c.oid) LEFT JOIN
pg_class d ON d.reltoastrelid=c.oid LEFT JOIN pg_database db ON
db.oid=b.reldatabase GROUP BY 2,3,4 ORDER BY 1 DESC LIMIT 22;Unpatched:
5000 | postgres | pg_toast_96188840 | t
=> 40MB of shared buffersPatched:
2048 | postgres | pg_toast_17097 | tNote that a similar problem seems to exist in COPY ... but I can't see
how to fix that one.--
Justin
--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/
Re: ALTER TABLE uses a bistate but not for toast tables
Hi Justin,
This patch has gone stale quite some time ago; CFbot does not seem to
have any history of a successful apply attemps, nor do we have any
succesful build history (which was introduced some time ago already).
Are you planning on rebasing this patch?
Kind regards,
Matthias van de Meent
Re: ALTER TABLE uses a bistate but not for toast tables
@cfbot: rebased
Attachments:
v4-0001-WIP-use-BulkInsertState-for-toast-tuples-too.patchtext/x-diff; charset=us-asciiDownload
From a9bb61421c24bd3273a8519362febb4073c297a1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 21 Jun 2022 22:28:06 -0500
Subject: [PATCH v4] WIP: use BulkInsertState for toast tuples, too
DONE: ALTER, CLUSTER
TODO: copyto, copyfrom?
slot_getsomeattrs slot_deform_heap_tuple fetchatt
heap_getnextslot => heapgettup => heapgetpage => ReadBufferExtended
initscan
table_beginscan table_scan_getnextslot
RelationCopyStorageUsingBuffer ReadBufferWithoutRelcache
(gdb) bt
#0 table_open (relationId=relationId@entry=16390, lockmode=lockmode@entry=1) at table.c:40
#1 0x000056444cb23d3c in toast_fetch_datum (attr=attr@entry=0x7f67933cc6cc) at detoast.c:372
#2 0x000056444cb24217 in detoast_attr (attr=attr@entry=0x7f67933cc6cc) at detoast.c:123
#3 0x000056444d07a4c8 in pg_detoast_datum_packed (datum=datum@entry=0x7f67933cc6cc) at fmgr.c:1743
#4 0x000056444d042c8d in text_to_cstring (t=0x7f67933cc6cc) at varlena.c:224
#5 0x000056444d0434f9 in textout (fcinfo=<optimized out>) at varlena.c:573
#6 0x000056444d078f10 in FunctionCall1Coll (flinfo=flinfo@entry=0x56444e4706b0, collation=collation@entry=0, arg1=arg1@entry=140082828592844) at fmgr.c:1124
#7 0x000056444d079d7f in OutputFunctionCall (flinfo=flinfo@entry=0x56444e4706b0, val=val@entry=140082828592844) at fmgr.c:1561
#8 0x000056444ccb1665 in CopyOneRowTo (cstate=cstate@entry=0x56444e470898, slot=slot@entry=0x56444e396d20) at copyto.c:975
#9 0x000056444ccb2c7d in DoCopyTo (cstate=cstate@entry=0x56444e470898) at copyto.c:891
#10 0x000056444ccab4c2 in DoCopy (pstate=pstate@entry=0x56444e396bb0, stmt=stmt@entry=0x56444e3759b0, stmt_location=0, stmt_len=48, processed=processed@entry=0x7ffc212a6310) at copy.c:308
cluster:
heapam_relation_copy_for_cluster
reform_and_rewrite_tuple
rewrite_heap_tuple
raw_heap_insert
//-os-only: linux
---
src/backend/access/common/toast_internals.c | 6 ++++--
src/backend/access/heap/heapam.c | 24 +++++++++++++++------
src/backend/access/heap/heaptoast.c | 11 ++++++----
src/backend/access/heap/rewriteheap.c | 6 +++++-
src/backend/access/table/toast_helper.c | 6 ++++--
src/include/access/heaptoast.h | 4 +++-
src/include/access/hio.h | 2 ++
src/include/access/toast_helper.h | 3 ++-
src/include/access/toast_internals.h | 4 +++-
9 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 371515395a5..3b52cba5eb1 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -118,7 +118,8 @@ toast_compress_datum(Datum value, char cmethod)
*/
Datum
toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options)
+ struct varlena *oldexternal, int options,
+ BulkInsertState bistate)
{
Relation toastrel;
Relation *toastidxs;
@@ -321,7 +322,8 @@ toast_save_datum(Relation rel, Datum value,
memcpy(VARDATA(&chunk_data), data_p, chunk_size);
toasttup = heap_form_tuple(toasttupDesc, t_values, t_isnull);
- heap_insert(toastrel, toasttup, mycid, options, NULL);
+ heap_insert(toastrel, toasttup, mycid, options, bistate ?
+ bistate->toast_state : NULL);
/*
* Create the index entry. We cheat a little here by not using
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 14de8158d49..470ffa7e708 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -74,7 +74,8 @@
static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
- TransactionId xid, CommandId cid, int options);
+ TransactionId xid, CommandId cid, int options,
+ BulkInsertState bistate);
static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tuple,
@@ -1765,9 +1766,15 @@ GetBulkInsertState(void)
bistate = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
bistate->strategy = GetAccessStrategy(BAS_BULKWRITE);
bistate->current_buf = InvalidBuffer;
+
bistate->next_free = InvalidBlockNumber;
bistate->last_free = InvalidBlockNumber;
bistate->already_extended_by = 0;
+
+ bistate->toast_state = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
+ bistate->toast_state->strategy = GetAccessStrategy(BAS_BULKWRITE);
+ bistate->toast_state->current_buf = InvalidBuffer;
+
return bistate;
}
@@ -1779,6 +1786,10 @@ FreeBulkInsertState(BulkInsertState bistate)
{
if (bistate->current_buf != InvalidBuffer)
ReleaseBuffer(bistate->current_buf);
+ if (bistate->toast_state->current_buf != InvalidBuffer)
+ ReleaseBuffer(bistate->toast_state->current_buf);
+
+ FreeAccessStrategy(bistate->toast_state->strategy);
FreeAccessStrategy(bistate->strategy);
pfree(bistate);
}
@@ -1844,7 +1855,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* Note: below this point, heaptup is the data we actually intend to store
* into the relation; tup is the caller's original untoasted data.
*/
- heaptup = heap_prepare_insert(relation, tup, xid, cid, options);
+
+ heaptup = heap_prepare_insert(relation, tup, xid, cid, options, bistate);
/*
* Find buffer to insert this tuple into. If the page is all visible,
@@ -2015,7 +2027,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
*/
static HeapTuple
heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
- CommandId cid, int options)
+ CommandId cid, int options, BulkInsertState bistate)
{
/*
* To allow parallel inserts, we need to ensure that they are safe to be
@@ -2051,7 +2063,7 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
return tup;
}
else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
- return heap_toast_insert_or_update(relation, tup, NULL, options);
+ return heap_toast_insert_or_update(relation, tup, NULL, options, bistate);
else
return tup;
}
@@ -2129,7 +2141,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
slots[i]->tts_tableOid = RelationGetRelid(relation);
tuple->t_tableOid = slots[i]->tts_tableOid;
heaptuples[i] = heap_prepare_insert(relation, tuple, xid, cid,
- options);
+ options, NULL);
}
/*
@@ -3594,7 +3606,7 @@ l2:
if (need_toast)
{
/* Note we always use WAL and FSM during updates */
- heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0);
+ heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0, NULL);
newtupsize = MAXALIGN(heaptup->t_len);
}
else
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index 52ecd45654c..74714b47ea0 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -94,7 +94,7 @@ heap_toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative)
*/
HeapTuple
heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
- int options)
+ int options, BulkInsertState bistate)
{
HeapTuple result_tuple;
TupleDesc tupleDesc;
@@ -110,6 +110,9 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
ToastAttrInfo toast_attr[MaxHeapAttributeNumber];
ToastTupleContext ttc;
+ /* Bulk insert is not supported for updates, only inserts. */
+ Assert(bistate == NULL || oldtup == NULL);
+
/*
* Ignore the INSERT_SPECULATIVE option. Speculative insertions/super
* deletions just normally insert/delete the toast values. It seems
@@ -214,7 +217,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
*/
if (toast_attr[biggest_attno].tai_size > maxDataLen &&
rel->rd_rel->reltoastrelid != InvalidOid)
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -231,7 +234,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
biggest_attno = toast_tuple_find_biggest_attribute(&ttc, false, false);
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -267,7 +270,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 424958912c7..afdf105b275 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -152,6 +152,7 @@ typedef struct RewriteStateData
HTAB *rs_old_new_tid_map; /* unmatched B tuples */
HTAB *rs_logical_mappings; /* logical remapping files */
uint32 rs_num_rewrite_mappings; /* # in memory mappings */
+ BulkInsertState rs_bistate;
} RewriteStateData;
/*
@@ -263,6 +264,7 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
state->rs_freeze_xid = freeze_xid;
state->rs_cutoff_multi = cutoff_multi;
state->rs_cxt = rw_cxt;
+ state->rs_bistate = GetBulkInsertState();
/* Initialize hash tables used to track update chains */
hash_ctl.keysize = sizeof(TidHashKey);
@@ -313,6 +315,8 @@ end_heap_rewrite(RewriteState state)
raw_heap_insert(state, unresolved->tuple);
}
+ FreeBulkInsertState(state->rs_bistate);
+
/* Write the last page, if any */
if (state->rs_buffer_valid)
{
@@ -643,7 +647,7 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
options |= HEAP_INSERT_NO_LOGICAL;
heaptup = heap_toast_insert_or_update(state->rs_new_rel, tup, NULL,
- options);
+ options, state->rs_bistate);
}
else
heaptup = tup;
diff --git a/src/backend/access/table/toast_helper.c b/src/backend/access/table/toast_helper.c
index 871ebeeb563..89ad027d47d 100644
--- a/src/backend/access/table/toast_helper.c
+++ b/src/backend/access/table/toast_helper.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include "access/detoast.h"
+#include "access/hio.h"
#include "access/table.h"
#include "access/toast_helper.h"
#include "access/toast_internals.h"
@@ -254,7 +255,8 @@ toast_tuple_try_compression(ToastTupleContext *ttc, int attribute)
* Move an attribute to external storage.
*/
void
-toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
+toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options,
+ BulkInsertStateData *bistate)
{
Datum *value = &ttc->ttc_values[attribute];
Datum old_value = *value;
@@ -262,7 +264,7 @@ toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
attr->tai_colflags |= TOASTCOL_IGNORE;
*value = toast_save_datum(ttc->ttc_rel, old_value, attr->tai_oldexternal,
- options);
+ options, bistate);
if ((attr->tai_colflags & TOASTCOL_NEEDS_FREE) != 0)
pfree(DatumGetPointer(old_value));
attr->tai_colflags |= TOASTCOL_NEEDS_FREE;
diff --git a/src/include/access/heaptoast.h b/src/include/access/heaptoast.h
index 5c0a796f661..61359ead73d 100644
--- a/src/include/access/heaptoast.h
+++ b/src/include/access/heaptoast.h
@@ -13,6 +13,7 @@
#ifndef HEAPTOAST_H
#define HEAPTOAST_H
+#include "access/hio.h"
#include "access/htup_details.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -95,7 +96,8 @@
* ----------
*/
extern HeapTuple heap_toast_insert_or_update(Relation rel, HeapTuple newtup,
- HeapTuple oldtup, int options);
+ HeapTuple oldtup, int options,
+ BulkInsertStateData *bistate);
/* ----------
* heap_toast_delete -
diff --git a/src/include/access/hio.h b/src/include/access/hio.h
index 9bc563b7628..3ab5f3e8d2a 100644
--- a/src/include/access/hio.h
+++ b/src/include/access/hio.h
@@ -48,6 +48,8 @@ typedef struct BulkInsertStateData
BlockNumber next_free;
BlockNumber last_free;
uint32 already_extended_by;
+
+ struct BulkInsertStateData *toast_state;
} BulkInsertStateData;
diff --git a/src/include/access/toast_helper.h b/src/include/access/toast_helper.h
index 51a228db402..260c9813c2b 100644
--- a/src/include/access/toast_helper.h
+++ b/src/include/access/toast_helper.h
@@ -14,6 +14,7 @@
#ifndef TOAST_HELPER_H
#define TOAST_HELPER_H
+#include "access/hio.h"
#include "utils/rel.h"
/*
@@ -107,7 +108,7 @@ extern int toast_tuple_find_biggest_attribute(ToastTupleContext *ttc,
bool check_main);
extern void toast_tuple_try_compression(ToastTupleContext *ttc, int attribute);
extern void toast_tuple_externalize(ToastTupleContext *ttc, int attribute,
- int options);
+ int options, BulkInsertStateData *bistate);
extern void toast_tuple_cleanup(ToastTupleContext *ttc);
extern void toast_delete_external(Relation rel, const Datum *values, const bool *isnull,
diff --git a/src/include/access/toast_internals.h b/src/include/access/toast_internals.h
index 30dba098936..95b15ed3d13 100644
--- a/src/include/access/toast_internals.h
+++ b/src/include/access/toast_internals.h
@@ -12,6 +12,7 @@
#ifndef TOAST_INTERNALS_H
#define TOAST_INTERNALS_H
+#include "access/hio.h"
#include "access/toast_compression.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -50,7 +51,8 @@ extern Oid toast_get_valid_index(Oid toastoid, LOCKMODE lock);
extern void toast_delete_datum(Relation rel, Datum value, bool is_speculative);
extern Datum toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options);
+ struct varlena *oldexternal, int options,
+ BulkInsertStateData *bistate);
extern int toast_open_indexes(Relation toastrel,
LOCKMODE lock,
--
2.42.0
Re: ALTER TABLE uses a bistate but not for toast tables
@cfbot: rebased
Attachments:
0001-WIP-use-BulkInsertState-for-toast-tuples-too.patchtext/x-diff; charset=us-asciiDownload
From bc92a05ba35115ba0df278d553a5c0e4e303fe23 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 21 Jun 2022 22:28:06 -0500
Subject: [PATCH] WIP: use BulkInsertState for toast tuples, too
DONE: ALTER, CLUSTER
TODO: copyto, copyfrom?
slot_getsomeattrs slot_deform_heap_tuple fetchatt
heap_getnextslot => heapgettup => heapgetpage => ReadBufferExtended
initscan
table_beginscan table_scan_getnextslot
RelationCopyStorageUsingBuffer ReadBufferWithoutRelcache
(gdb) bt
#0 table_open (relationId=relationId@entry=16390, lockmode=lockmode@entry=1) at table.c:40
#1 0x000056444cb23d3c in toast_fetch_datum (attr=attr@entry=0x7f67933cc6cc) at detoast.c:372
#2 0x000056444cb24217 in detoast_attr (attr=attr@entry=0x7f67933cc6cc) at detoast.c:123
#3 0x000056444d07a4c8 in pg_detoast_datum_packed (datum=datum@entry=0x7f67933cc6cc) at fmgr.c:1743
#4 0x000056444d042c8d in text_to_cstring (t=0x7f67933cc6cc) at varlena.c:224
#5 0x000056444d0434f9 in textout (fcinfo=<optimized out>) at varlena.c:573
#6 0x000056444d078f10 in FunctionCall1Coll (flinfo=flinfo@entry=0x56444e4706b0, collation=collation@entry=0, arg1=arg1@entry=140082828592844) at fmgr.c:1124
#7 0x000056444d079d7f in OutputFunctionCall (flinfo=flinfo@entry=0x56444e4706b0, val=val@entry=140082828592844) at fmgr.c:1561
#8 0x000056444ccb1665 in CopyOneRowTo (cstate=cstate@entry=0x56444e470898, slot=slot@entry=0x56444e396d20) at copyto.c:975
#9 0x000056444ccb2c7d in DoCopyTo (cstate=cstate@entry=0x56444e470898) at copyto.c:891
#10 0x000056444ccab4c2 in DoCopy (pstate=pstate@entry=0x56444e396bb0, stmt=stmt@entry=0x56444e3759b0, stmt_location=0, stmt_len=48, processed=processed@entry=0x7ffc212a6310) at copy.c:308
cluster:
heapam_relation_copy_for_cluster
reform_and_rewrite_tuple
rewrite_heap_tuple
raw_heap_insert
//-os-only: linux
---
src/backend/access/common/toast_internals.c | 6 ++++--
src/backend/access/heap/heapam.c | 24 +++++++++++++++------
src/backend/access/heap/heaptoast.c | 11 ++++++----
src/backend/access/heap/rewriteheap.c | 8 +++++--
src/backend/access/table/toast_helper.c | 6 ++++--
src/include/access/heaptoast.h | 4 +++-
src/include/access/hio.h | 2 ++
src/include/access/toast_helper.h | 3 ++-
src/include/access/toast_internals.h | 4 +++-
9 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 90d0654e629..b66d32a26e3 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -117,7 +117,8 @@ toast_compress_datum(Datum value, char cmethod)
*/
Datum
toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options)
+ struct varlena *oldexternal, int options,
+ BulkInsertState bistate)
{
Relation toastrel;
Relation *toastidxs;
@@ -320,7 +321,8 @@ toast_save_datum(Relation rel, Datum value,
memcpy(VARDATA(&chunk_data), data_p, chunk_size);
toasttup = heap_form_tuple(toasttupDesc, t_values, t_isnull);
- heap_insert(toastrel, toasttup, mycid, options, NULL);
+ heap_insert(toastrel, toasttup, mycid, options, bistate ?
+ bistate->toast_state : NULL);
/*
* Create the index entry. We cheat a little here by not using
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 91b20147a00..18b05d46735 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -71,7 +71,8 @@
static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
- TransactionId xid, CommandId cid, int options);
+ TransactionId xid, CommandId cid, int options,
+ BulkInsertState bistate);
static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tuple,
@@ -1931,9 +1932,15 @@ GetBulkInsertState(void)
bistate = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
bistate->strategy = GetAccessStrategy(BAS_BULKWRITE);
bistate->current_buf = InvalidBuffer;
+
bistate->next_free = InvalidBlockNumber;
bistate->last_free = InvalidBlockNumber;
bistate->already_extended_by = 0;
+
+ bistate->toast_state = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
+ bistate->toast_state->strategy = GetAccessStrategy(BAS_BULKWRITE);
+ bistate->toast_state->current_buf = InvalidBuffer;
+
return bistate;
}
@@ -1945,6 +1952,10 @@ FreeBulkInsertState(BulkInsertState bistate)
{
if (bistate->current_buf != InvalidBuffer)
ReleaseBuffer(bistate->current_buf);
+ if (bistate->toast_state->current_buf != InvalidBuffer)
+ ReleaseBuffer(bistate->toast_state->current_buf);
+
+ FreeAccessStrategy(bistate->toast_state->strategy);
FreeAccessStrategy(bistate->strategy);
pfree(bistate);
}
@@ -2010,7 +2021,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* Note: below this point, heaptup is the data we actually intend to store
* into the relation; tup is the caller's original untoasted data.
*/
- heaptup = heap_prepare_insert(relation, tup, xid, cid, options);
+
+ heaptup = heap_prepare_insert(relation, tup, xid, cid, options, bistate);
/*
* Find buffer to insert this tuple into. If the page is all visible,
@@ -2181,7 +2193,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
*/
static HeapTuple
heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
- CommandId cid, int options)
+ CommandId cid, int options, BulkInsertState bistate)
{
/*
* To allow parallel inserts, we need to ensure that they are safe to be
@@ -2217,7 +2229,7 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
return tup;
}
else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
- return heap_toast_insert_or_update(relation, tup, NULL, options);
+ return heap_toast_insert_or_update(relation, tup, NULL, options, bistate);
else
return tup;
}
@@ -2295,7 +2307,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
slots[i]->tts_tableOid = RelationGetRelid(relation);
tuple->t_tableOid = slots[i]->tts_tableOid;
heaptuples[i] = heap_prepare_insert(relation, tuple, xid, cid,
- options);
+ options, NULL);
}
/*
@@ -3767,7 +3779,7 @@ l2:
if (need_toast)
{
/* Note we always use WAL and FSM during updates */
- heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0);
+ heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0, NULL);
newtupsize = MAXALIGN(heaptup->t_len);
}
else
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index a420e165304..bcb012c579d 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -94,7 +94,7 @@ heap_toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative)
*/
HeapTuple
heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
- int options)
+ int options, BulkInsertState bistate)
{
HeapTuple result_tuple;
TupleDesc tupleDesc;
@@ -110,6 +110,9 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
ToastAttrInfo toast_attr[MaxHeapAttributeNumber];
ToastTupleContext ttc;
+ /* Bulk insert is not supported for updates, only inserts. */
+ Assert(bistate == NULL || oldtup == NULL);
+
/*
* Ignore the INSERT_SPECULATIVE option. Speculative insertions/super
* deletions just normally insert/delete the toast values. It seems
@@ -214,7 +217,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
*/
if (toast_attr[biggest_attno].tai_size > maxDataLen &&
rel->rd_rel->reltoastrelid != InvalidOid)
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -231,7 +234,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
biggest_attno = toast_tuple_find_biggest_attribute(&ttc, false, false);
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
@@ -267,7 +270,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
if (biggest_attno < 0)
break;
- toast_tuple_externalize(&ttc, biggest_attno, options);
+ toast_tuple_externalize(&ttc, biggest_attno, options, bistate);
}
/*
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 473f3aa9bef..d313ad64ee7 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -150,6 +150,7 @@ typedef struct RewriteStateData
HTAB *rs_old_new_tid_map; /* unmatched B tuples */
HTAB *rs_logical_mappings; /* logical remapping files */
uint32 rs_num_rewrite_mappings; /* # in memory mappings */
+ BulkInsertState rs_bistate;
} RewriteStateData;
/*
@@ -260,7 +261,8 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
state->rs_freeze_xid = freeze_xid;
state->rs_cutoff_multi = cutoff_multi;
state->rs_cxt = rw_cxt;
- state->rs_bulkstate = smgr_bulk_start_rel(new_heap, MAIN_FORKNUM);
+ state->rs_bulkstate = smgr_bulk_start_rel(new_heap, MAIN_FORKNUM); /* This is a BulkWriteState */
+ state->rs_bistate = GetBulkInsertState(); /* This is a BulkInsertState ... */
/* Initialize hash tables used to track update chains */
hash_ctl.keysize = sizeof(TidHashKey);
@@ -311,6 +313,8 @@ end_heap_rewrite(RewriteState state)
raw_heap_insert(state, unresolved->tuple);
}
+ FreeBulkInsertState(state->rs_bistate);
+
/* Write the last page, if any */
if (state->rs_buffer)
{
@@ -624,7 +628,7 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
options |= HEAP_INSERT_NO_LOGICAL;
heaptup = heap_toast_insert_or_update(state->rs_new_rel, tup, NULL,
- options);
+ options, state->rs_bistate);
}
else
heaptup = tup;
diff --git a/src/backend/access/table/toast_helper.c b/src/backend/access/table/toast_helper.c
index 53224932f0d..2adf2dfaa9a 100644
--- a/src/backend/access/table/toast_helper.c
+++ b/src/backend/access/table/toast_helper.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include "access/detoast.h"
+#include "access/hio.h"
#include "access/toast_helper.h"
#include "access/toast_internals.h"
#include "catalog/pg_type_d.h"
@@ -253,7 +254,8 @@ toast_tuple_try_compression(ToastTupleContext *ttc, int attribute)
* Move an attribute to external storage.
*/
void
-toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
+toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options,
+ BulkInsertStateData *bistate)
{
Datum *value = &ttc->ttc_values[attribute];
Datum old_value = *value;
@@ -261,7 +263,7 @@ toast_tuple_externalize(ToastTupleContext *ttc, int attribute, int options)
attr->tai_colflags |= TOASTCOL_IGNORE;
*value = toast_save_datum(ttc->ttc_rel, old_value, attr->tai_oldexternal,
- options);
+ options, bistate);
if ((attr->tai_colflags & TOASTCOL_NEEDS_FREE) != 0)
pfree(DatumGetPointer(old_value));
attr->tai_colflags |= TOASTCOL_NEEDS_FREE;
diff --git a/src/include/access/heaptoast.h b/src/include/access/heaptoast.h
index c376dff48d7..a993aea5857 100644
--- a/src/include/access/heaptoast.h
+++ b/src/include/access/heaptoast.h
@@ -13,6 +13,7 @@
#ifndef HEAPTOAST_H
#define HEAPTOAST_H
+#include "access/hio.h"
#include "access/htup_details.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -95,7 +96,8 @@
* ----------
*/
extern HeapTuple heap_toast_insert_or_update(Relation rel, HeapTuple newtup,
- HeapTuple oldtup, int options);
+ HeapTuple oldtup, int options,
+ BulkInsertStateData *bistate);
/* ----------
* heap_toast_delete -
diff --git a/src/include/access/hio.h b/src/include/access/hio.h
index 24621cf22b2..e47fefe2ab9 100644
--- a/src/include/access/hio.h
+++ b/src/include/access/hio.h
@@ -48,6 +48,8 @@ typedef struct BulkInsertStateData
BlockNumber next_free;
BlockNumber last_free;
uint32 already_extended_by;
+
+ struct BulkInsertStateData *toast_state;
} BulkInsertStateData;
diff --git a/src/include/access/toast_helper.h b/src/include/access/toast_helper.h
index 349a513f77d..04ff56b7cc7 100644
--- a/src/include/access/toast_helper.h
+++ b/src/include/access/toast_helper.h
@@ -14,6 +14,7 @@
#ifndef TOAST_HELPER_H
#define TOAST_HELPER_H
+#include "access/hio.h"
#include "utils/rel.h"
/*
@@ -107,7 +108,7 @@ extern int toast_tuple_find_biggest_attribute(ToastTupleContext *ttc,
bool check_main);
extern void toast_tuple_try_compression(ToastTupleContext *ttc, int attribute);
extern void toast_tuple_externalize(ToastTupleContext *ttc, int attribute,
- int options);
+ int options, BulkInsertStateData *bistate);
extern void toast_tuple_cleanup(ToastTupleContext *ttc);
extern void toast_delete_external(Relation rel, const Datum *values, const bool *isnull,
diff --git a/src/include/access/toast_internals.h b/src/include/access/toast_internals.h
index 0eeefe59fec..1be114bf24f 100644
--- a/src/include/access/toast_internals.h
+++ b/src/include/access/toast_internals.h
@@ -12,6 +12,7 @@
#ifndef TOAST_INTERNALS_H
#define TOAST_INTERNALS_H
+#include "access/hio.h"
#include "access/toast_compression.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
@@ -50,7 +51,8 @@ extern Oid toast_get_valid_index(Oid toastoid, LOCKMODE lock);
extern void toast_delete_datum(Relation rel, Datum value, bool is_speculative);
extern Datum toast_save_datum(Relation rel, Datum value,
- struct varlena *oldexternal, int options);
+ struct varlena *oldexternal, int options,
+ BulkInsertStateData *bistate);
extern int toast_open_indexes(Relation toastrel,
LOCKMODE lock,
--
2.42.0
Re: ALTER TABLE uses a bistate but not for toast tables
On Mon, Jul 15, 2024 at 03:43:24PM GMT, Justin Pryzby wrote:
@cfbot: rebased
Hey Justin,
Thanks for rebasing. To help with review, could you also describe
current status of the patch? I have to admit, currently the commit
message doesn't tell much, and looks more like notes for the future you.
The patch numbering is somewhat confusing as well, should it be v5 now?
From what I understand, the new patch does address the review feedback,
but you want to do more, something with copy to / copy from?
Since it's in the performance category, I'm also curious how much
overhead does this shave off? I mean, I get it that bulk insert strategy
helps with buffers usage, as you've implied in the thread -- but how
does it look like in benchmark numbers?
Re: ALTER TABLE uses a bistate but not for toast tables
On Tue, Nov 19, 2024 at 03:45:19PM +0100, Dmitry Dolgov wrote:
On Mon, Jul 15, 2024 at 03:43:24PM GMT, Justin Pryzby wrote:
@cfbot: rebasedThanks for rebasing. To help with review, could you also describe
current status of the patch? I have to admit, currently the commit
message doesn't tell much, and looks more like notes for the future you.
The patch does what it aims to do and AFAIK in a reasonable way. I'm
not aware of any issue with it. It's, uh, waiting for review.
I'm happy to expand on the message to describe something like design
choices, but the goal here is really simple: why should wide column
values escape the intention of the ring buffer? AFAICT it's fixing an
omission. If you have a question, please ask; that would help to
indicate what needs to be explained.
The patch numbering is somewhat confusing as well, should it be v5 now?
The filename was 0001-WIP-use-BulkInsertState-for-toast-tuples-too.patch.
I guess you're referring to the previous filename: v4-*.
That shouldn't be so confusing -- I just didn't specify a version,
either by choice or by omission.
From what I understand, the new patch does address the review feedback,
but you want to do more, something with copy to / copy from?
If I were to do more, it'd be for a future patch, if the current patch
were to ever progress.
Since it's in the performance category, I'm also curious how much
overhead does this shave off? I mean, I get it that bulk insert strategy
helps with buffers usage, as you've implied in the thread -- but how
does it look like in benchmark numbers?
The intent of using a bistate isn't to help the performance of the
process using the bistate. Rather, the intent is to avoid harming the
performance of other processes. If anything, I expect it could slow
down the process using bistate -- the same as for non-toast data.
/messages/by-id/CA+TgmobC6RD2N8kbPPTvATpUY1kisY2wJLh2jsg=HGoCp2RiXw@mail.gmail.com
--
Justin
Re: ALTER TABLE uses a bistate but not for toast tables
On Wed, Nov 20, 2024 at 06:43:58AM -0600, Justin Pryzby wrote:
Thanks for rebasing. To help with review, could you also describe
current status of the patch? I have to admit, currently the commit
message doesn't tell much, and looks more like notes for the future you.The patch does what it aims to do and AFAIK in a reasonable way. I'm
not aware of any issue with it. It's, uh, waiting for review.I'm happy to expand on the message to describe something like design
choices, but the goal here is really simple: why should wide column
values escape the intention of the ring buffer? AFAICT it's fixing an
omission. If you have a question, please ask; that would help to
indicate what needs to be explained.
Here is what I see in the commit message:
DONE: ALTER, CLUSTER
TODO: copyto, copyfrom?
slot_getsomeattrs slot_deform_heap_tuple fetchatt
heap_getnextslot => heapgettup => heapgetpage => ReadBufferExtended
initscan
table_beginscan table_scan_getnextslot
RelationCopyStorageUsingBuffer ReadBufferWithoutRelcache
(gdb) bt
#0 table_open (relationId=relationId@entry=16390, lockmode=lockmode@entry=1) at table.c:40
#1 0x000056444cb23d3c in toast_fetch_datum (attr=attr@entry=0x7f67933cc6cc) at detoast.c:372
#2 0x000056444cb24217 in detoast_attr (attr=attr@entry=0x7f67933cc6cc) at detoast.c:123
#3 0x000056444d07a4c8 in pg_detoast_datum_packed (datum=datum@entry=0x7f67933cc6cc) at fmgr.c:1743
#4 0x000056444d042c8d in text_to_cstring (t=0x7f67933cc6cc) at varlena.c:224
#5 0x000056444d0434f9 in textout (fcinfo=<optimized out>) at varlena.c:573
#6 0x000056444d078f10 in FunctionCall1Coll (flinfo=flinfo@entry=0x56444e4706b0, collation=collation@entry=0, arg1=arg1@entry=140082828592844) at fmgr.c:1124
#7 0x000056444d079d7f in OutputFunctionCall (flinfo=flinfo@entry=0x56444e4706b0, val=val@entry=140082828592844) at fmgr.c:1561
#8 0x000056444ccb1665 in CopyOneRowTo (cstate=cstate@entry=0x56444e470898, slot=slot@entry=0x56444e396d20) at copyto.c:975
#9 0x000056444ccb2c7d in DoCopyTo (cstate=cstate@entry=0x56444e470898) at copyto.c:891
#10 0x000056444ccab4c2 in DoCopy (pstate=pstate@entry=0x56444e396bb0, stmt=stmt@entry=0x56444e3759b0, stmt_location=0, stmt_len=48, processed=processed@entry=0x7ffc212a6310) at copy.c:308
cluster:
heapam_relation_copy_for_cluster
reform_and_rewrite_tuple
rewrite_heap_tuple
raw_heap_insert
This gave me an impression, that the patch is deeply WIP, and it doesn't
make any sense to review it. I can imagine chances are good, that I'm
not alone who get such impression, and you loose potential reviewers.
Thus, shaping up a meaningful message might be helpful.
Since it's in the performance category, I'm also curious how much
overhead does this shave off? I mean, I get it that bulk insert strategy
helps with buffers usage, as you've implied in the thread -- but how
does it look like in benchmark numbers?The intent of using a bistate isn't to help the performance of the
process using the bistate. Rather, the intent is to avoid harming the
performance of other processes. If anything, I expect it could slow
down the process using bistate -- the same as for non-toast data./messages/by-id/CA+TgmobC6RD2N8kbPPTvATpUY1kisY2wJLh2jsg=HGoCp2RiXw@mail.gmail.com
Right, but the question is still there, how much does it bring? My point
is that if you demonstrate "under this and that load, we get so and so
many percents boost", this will hopefully attract more attention to the
patch.