agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Cc: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Robert Treat <rob@xzilla.net>
Subject: Re: Adding REPACK [concurrently]
Date: Thu, 26 Mar 2026 15:06:29 +0100
Message-ID: <43861.1774533989@localhost> (raw)
In-Reply-To: <202603252005.quy5h4oipoxd@alvherre.pgsql>
References: <202603252005.quy5h4oipoxd@alvherre.pgsql>
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> (I omitted the last three patches in the series, and
> squashed my proposed changes into 0003, as announced in my previous
> posting.)
I've updated the comment about on-disk attributes in repack_store_change(),
but when verifying it, I hit an error when more than one UPDATEs (in separate
transactions) were executed during a single run of REPACK.
The problem is that reorderbuffer.c sets up an internal (sub)transaction
before replaying each decoded transaction. Therefore the tuple slot should not
be allocated in TopTransactionContext. I chose TopMemoryContext instead.
BTW, if you want to verify that the updated comment is correct, just add
elog(ERROR) next to it and run repack_toast.spec. The statement
UPDATE repack_test SET i=3 where i=1;
will then reach it.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
[text/x-diff] nocfbot_fix_repack_store_change.diff (1.3K, ../43861.1774533989@localhost/2-nocfbot_fix_repack_store_change.diff)
download | inline diff:
diff --git a/src/backend/replication/pgoutput_repack/pgoutput_repack.c b/src/backend/replication/pgoutput_repack/pgoutput_repack.c
index cc9ce615b18..5fe3115509e 100644
--- a/src/backend/replication/pgoutput_repack/pgoutput_repack.c
+++ b/src/backend/replication/pgoutput_repack/pgoutput_repack.c
@@ -202,7 +202,11 @@ repack_store_change(LogicalDecodingContext *ctx, Relation relation,
/* Initialize the slot, if not done already */
if (dstate->slot == NULL)
{
- MemoryContextSwitchTo(oldcxt);
+ /*
+ * We are in the decoding worker, so no worries about polluting
+ * memory of the backend executing REPACK.
+ */
+ MemoryContextSwitchTo(TopMemoryContext);
dstate->slot = MakeSingleTupleTableSlot(desc, &TTSOpsHeapTuple);
MemoryContextSwitchTo(dstate->change_cxt);
}
@@ -247,8 +251,8 @@ repack_store_change(LogicalDecodingContext *ctx, Relation relation,
* attributes (those actually should never appear on disk), so
* only TOASTed attribute can be seen here.
*
- * FIXME in what circumstances can an ONDISK attr appear? Why
- * aren't these written separately?
+ * We get here if the table has external values but only
+ * in-line values are being updated now.
*/
Assert(VARATT_IS_EXTERNAL_ONDISK(varlen));
}
view thread (416+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, alvherre@alvh.no-ip.org, srinath2133@gmail.com, mihailnikalayeu@gmail.com, boekewurm+postgres@gmail.com, pgsql-hackers@lists.postgresql.org, rob@xzilla.net
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <43861.1774533989@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox