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
>From 90449459a6d8c4f87bfe23f462a45649beed4a8d Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@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