public inbox for [email protected]
help / color / mirror / Atom feedRe: Adding REPACK [concurrently]
8+ messages / 4 participants
[nested] [flat]
* Re: Adding REPACK [concurrently]
@ 2026-02-24 18:29 Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Alvaro Herrera @ 2026-02-24 18:29 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
On 2026-Feb-23, Alvaro Herrera wrote:
Looking at this function in pgoutput_repack.c:
> +/* Store concurrent data change. */
> +static void
> +store_change(LogicalDecodingContext *ctx, ConcurrentChangeKind kind,
> + HeapTuple tuple)
> +{
[...] we have this:
> + size = VARHDRSZ + SizeOfConcurrentChange;
> +
> + /*
> + * ReorderBufferCommit() stores the TOAST chunks in its private memory
> + * context and frees them after having called apply_change(). Therefore
> + * we need flat copy (including TOAST) that we eventually copy into the
> + * memory context which is available to decode_concurrent_changes().
> + */
> + if (HeapTupleHasExternal(tuple))
> + {
> + /*
> + * toast_flatten_tuple_to_datum() might be more convenient but we
> + * don't want the decompression it does.
> + */
> + tuple = toast_flatten_tuple(tuple, dstate->tupdesc);
> + flattened = true;
> + }
> +
> + size += tuple->t_len;
> + if (size >= MaxAllocSize)
> + elog(ERROR, "Change is too big.");
> +
> + /* Construct the change. */
> + change_raw = (char *) palloc0(size);
> + SET_VARSIZE(change_raw, size);
I wonder if this isn't problematic with large tuples. If a row has some
very wide columns, each of which individually is less than 1 GB, then it
might happen that the sum of their sizes exceeds 1 GB, causing palloc()
to complain and abort the whole repack operation. This wouldn't be very
nice, so I think we need to address it somehow.
Another thing I'm not very keen on, is the fact that we have to memcpy()
the tuple contents a few lines below:
> + /*
> + * Copy the tuple.
> + *
> + * Note: change->tup_data.t_data must be fixed on retrieval!
> + */
> + memcpy(&change.tup_data, tuple, sizeof(HeapTupleData));
> + memcpy(dst, &change, SizeOfConcurrentChange);
> + dst += SizeOfConcurrentChange;
> + memcpy(dst, tuple->t_data, tuple->t_len);
> + /* Store as tuple of 1 bytea column. */
> + values[0] = PointerGetDatum(change_raw);
> + isnull[0] = false;
> + tuplestore_putvalues(dstate->tstore, dstate->tupdesc_change,
> + values, isnull);
To make matters worse, tuplestore_putvalues does a
heap_form_minimal_tuple() on this and copies the data again. This seems
pretty wasteful.
I think we need some new APIs to avoid all this copying. It appears
that it all starts with reorderbuffer doing something unhelpful with the
memory context of the TOAST chunks. Maybe we should address this by
"fixing" reorderbuffer so that it doesn't do this, instead of playing so
many games to cope.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
@ 2026-02-25 08:55 ` Antonin Houska <[email protected]>
2026-02-25 13:54 ` Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 13:55 ` Re: Adding REPACK [concurrently] Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Antonin Houska @ 2026-02-25 08:55 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Alvaro Herrera <[email protected]> wrote:
> On 2026-Feb-23, Alvaro Herrera wrote:
>
> Looking at this function in pgoutput_repack.c:
>
> > +/* Store concurrent data change. */
> > +static void
> > +store_change(LogicalDecodingContext *ctx, ConcurrentChangeKind kind,
> > + HeapTuple tuple)
> > +{
>
> [...] we have this:
>
> > + size = VARHDRSZ + SizeOfConcurrentChange;
> > +
> > + /*
> > + * ReorderBufferCommit() stores the TOAST chunks in its private memory
> > + * context and frees them after having called apply_change(). Therefore
> > + * we need flat copy (including TOAST) that we eventually copy into the
> > + * memory context which is available to decode_concurrent_changes().
> > + */
> > + if (HeapTupleHasExternal(tuple))
> > + {
> > + /*
> > + * toast_flatten_tuple_to_datum() might be more convenient but we
> > + * don't want the decompression it does.
> > + */
> > + tuple = toast_flatten_tuple(tuple, dstate->tupdesc);
> > + flattened = true;
> > + }
> > +
> > + size += tuple->t_len;
> > + if (size >= MaxAllocSize)
> > + elog(ERROR, "Change is too big.");
> > +
> > + /* Construct the change. */
> > + change_raw = (char *) palloc0(size);
> > + SET_VARSIZE(change_raw, size);
In 0005 ("Use background worker to do logical decoding"), the function is a
bit simpler because here the decoding worker uses temporary file to send the
data to the REPACKing backend, rather than tuplestore. sharedtuplestore.h
would also work but I think we do not need its functionality, and AFAICS it
always writes the data into a file anyway (i.e. it does not use memory even if
the amount of data is small).
(Perhaps 0004 should use the file too, in case 0005 does not make it into PG
19.)
> I wonder if this isn't problematic with large tuples. If a row has some
> very wide columns, each of which individually is less than 1 GB, then it
> might happen that the sum of their sizes exceeds 1 GB, causing palloc()
> to complain and abort the whole repack operation. This wouldn't be very
> nice, so I think we need to address it somehow.
I agree.
> I think we need some new APIs to avoid all this copying. It appears
> that it all starts with reorderbuffer doing something unhelpful with the
> memory context of the TOAST chunks. Maybe we should address this by
> "fixing" reorderbuffer so that it doesn't do this, instead of playing so
> many games to cope.
What I see is that reorderbuffer.c collects the TOAST pointers
(ReorderBufferToastAppendChunk) and then, before passing the tuple to the
output plugin, it copies the TOAST chunks referenced by the tuple to memory
and replaces the "on-disk" TOAST pointers in the tuple with "external
indirect" ones, pointing to the in-memory TOAST chunks
(ReorderBufferToastReplace).
For REPACK, I suggest a variant of toast_flatten_tuple() that writes the
output to a file, and a corresponding function that reads it while allocating
separate chunks of memory for the individual TOASTed attributes - the restored
tuple would reference the chunks using the "external indirect" TOAST pointers,
as if it had been processed by ReorderBufferToastReplace(). Does that make
sense to you?
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
@ 2026-02-25 13:54 ` Alvaro Herrera <[email protected]>
2026-02-25 16:03 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Alvaro Herrera @ 2026-02-25 13:54 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
On 2026-Feb-25, Antonin Houska wrote:
> In 0005 ("Use background worker to do logical decoding"), the function is a
> bit simpler because here the decoding worker uses temporary file to send the
> data to the REPACKing backend, rather than tuplestore.
Ah, that patch changes the implementation rather substantially then; I
didn't realize that because I haven't been looking at it yet, as I
wanted to have a good idea of the status of 0004 before proceeding
further with the next one. However, given that these changes are so
extensive, I think it might be better to review 0004+0005 squashed as
one unit, rather than separately.
> For REPACK, I suggest a variant of toast_flatten_tuple() that writes the
> output to a file, and a corresponding function that reads it while allocating
> separate chunks of memory for the individual TOASTed attributes - the restored
> tuple would reference the chunks using the "external indirect" TOAST pointers,
> as if it had been processed by ReorderBufferToastReplace(). Does that make
> sense to you?
Hmm, so on the apply side when reading the file, we would first reach
each toast attribute value, which we know to insert directly to the
toast table (keeping track of each individually toast pointer as we do
so); then we reach the heap tuple itself, we [... somehow ...] interpret
these external indirect toast pointers and substitute the toast pointers
that we created. So we never have to construct the entire tuple, or
indeed do anything else with the toasted values other than insert them
into the toast table.
Actually, can't we simply insert the toasted values directly in the
decoding worker into the new toast table? That could save a lot of
writing to the file, since we only save the raw heap tuples with no
toasted contents; but it's not clear to me that this is valid. (And we
might create extra bloat if a tuple is inserted and later deleted
concurrently with the repack; but that would happen with the original
approach as well, no?)
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"We’ve narrowed the problem down to the customer’s pants being in a situation
of vigorous combustion" (Robert Haas, Postgres expert extraordinaire)
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
2026-02-25 13:54 ` Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
@ 2026-02-25 16:03 ` Antonin Houska <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Antonin Houska @ 2026-02-25 16:03 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Alvaro Herrera <[email protected]> wrote:
> On 2026-Feb-25, Antonin Houska wrote:
> > For REPACK, I suggest a variant of toast_flatten_tuple() that writes the
> > output to a file, and a corresponding function that reads it while allocating
> > separate chunks of memory for the individual TOASTed attributes - the restored
> > tuple would reference the chunks using the "external indirect" TOAST pointers,
> > as if it had been processed by ReorderBufferToastReplace(). Does that make
> > sense to you?
>
> Hmm, so on the apply side when reading the file, we would first reach
> each toast attribute value, which we know to insert directly to the
> toast table (keeping track of each individually toast pointer as we do
> so); then we reach the heap tuple itself, we [... somehow ...] interpret
> these external indirect toast pointers and substitute the toast pointers
> that we created. So we never have to construct the entire tuple, or
> indeed do anything else with the toasted values other than insert them
> into the toast table.
Yes, that's what I mean.
> Actually, can't we simply insert the toasted values directly in the
> decoding worker into the new toast table? That could save a lot of
> writing to the file, since we only save the raw heap tuples with no
> toasted contents; but it's not clear to me that this is valid. (And we
> might create extra bloat if a tuple is inserted and later deleted
> concurrently with the repack; but that would happen with the original
> approach as well, no?)
The problem I see here is that for UPDATE you need the old tuple to determine
if its TOAST value should be deleted or if the new tuple should reuse it -
this is how I understand toast_tuple_init(). So the worker would have to store
all the changes somewhere temporarily until it can fully apply the changes
(i.e. until the initial copy and index build is complete).
Besides that, if the worker had to switch between the past (for the decoding)
and present (for the TOAST operations), it would have to invalidate system
caches repeatedly. 0004 does that, but 0005 makes that unnecessary. (I don't
know if the repeated cache invalidation would be a serious performance
problem, but from coding perspective I find it more convenient if the worker
only deals with decoding and does not have to do this time travel and
invalidations at all.)
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
@ 2026-02-25 13:55 ` Srinath Reddy Sadipiralla <[email protected]>
2026-02-25 19:41 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
1 sibling, 1 reply; 8+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-02-25 13:55 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Hello,
I did stress testing on v35 patches, where I did concurrency test using
pgbench with 50 concurrent clients, 4 threads with the below pgbench
script (dual_chaos.sql) on the following table setup(setup.sql).
I ran pgbench with 5M rows for 10 minutes and 50M for ~45 minutes
multiple times. REPACK (concurrently) ran successfully except "once"(see
below).
I created a shadow/clone table to use for checking the correctness after
doing
the concurrency test.I used 4 checks to verify that data is intact and
REPACK (concurrently) ran successfully.
1) table file OID(relfilenode) swapped?
2) bloat gone? victim relation size should be less than
shadow relation size.
3) using FULL JOIN logic (borrowed from repack.spec, with small change)
against the shadow table which goes under the same concurrent ops
done on the victim table , basically doing dual writes (see dual_chaos.sql)
to
verify table data integrity.
4) Physical Index Integrity (amcheck) (borrowed from Mihail's tests)
The concurrency test failed once. I tried to reproduce the below scenario
but no luck,i think the reason the assert failure happened because
after speculative insert there might be no spec CONFIRM or ABORT, thoughts?
TRAP: failed Assert("!specinsert"), File: "reorderbuffer.c", Line: 2610,
PID: 3956168
postgres: REPACK decoding worker for relation "stress_victim"
(ExceptionalCondition+0x98)[0xaaaab1251188]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x67b1cc)[0xaaaab0f4b1cc]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x67b86c)[0xaaaab0f4b86c]
postgres: REPACK decoding worker for relation "stress_victim"
(ReorderBufferCommit+0x74)[0xaaaab0f4b8f0]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x66229c)[0xaaaab0f3229c]
postgres: REPACK decoding worker for relation "stress_victim"
(xact_decode+0x1a0)[0xaaaab0f312bc]
postgres: REPACK decoding worker for relation "stress_victim"
(LogicalDecodingProcessRecord+0xd4)[0xaaaab0f30e60]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x3372e4)[0xaaaab0c072e4]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x339634)[0xaaaab0c09634]
postgres: REPACK decoding worker for relation "stress_victim"
(RepackWorkerMain+0x1ac)[0xaaaab0c094e8]
postgres: REPACK decoding worker for relation "stress_victim"
(BackgroundWorkerMain+0x2b0)[0xaaaab0efc440]
postgres: REPACK decoding worker for relation "stress_victim"
(postmaster_child_launch+0x1f0)[0xaaaab0f00398]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x639ca4)[0xaaaab0f09ca4]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x639f94)[0xaaaab0f09f94]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x638714)[0xaaaab0f08714]
postgres: REPACK decoding worker for relation "stress_victim"
(+0x635978)[0xaaaab0f05978]
postgres: REPACK decoding worker for relation "stress_victim"
(PostmasterMain+0x160c)[0xaaaab0f050c8]
postgres: REPACK decoding worker for relation "stress_victim"
(main+0x3dc)[0xaaaab0d974d4]
/lib/aarch64-linux-gnu/libc.so.6(+0x284c4)[0xffff867584c4]
/lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0x98)[0xffff86758598]
postgres: REPACK decoding worker for relation "stress_victim"
(_start+0x30)[0xaaaab09bc1f0]
2026-02-19 18:20:56.088 IST [3905812] LOG: checkpoint starting: wal
2026-02-19 18:21:10.683 IST [3905808] LOG: background worker "REPACK
decoding worker" (PID 3956168) was terminated by signal 6: Aborted
Crash Test:
i did crash test using debugger using a breakpoint
inside apply_concurrent_changes
to simulate a crash while concurrent changes are being done, after few
concurrent changes
are done , i crashed the server using "pg_ctl -m immediate stop", then
restarted the server,
i observed that REPACK (concurrently) didn't completed (expected), files
were not swapped and data
on the victim table is intact checked using FULL JOIN with shadow table,
but there are
some leftovers of the transient table we used for REPACK (concurrently)
such as
1) transient table's relation files - these consume extra space , i think
this was the
case with VACUUM FULL previously, so these has to be removed manually , but
I think this time we have a "leverage" which we can use to remove the extra
space.
2) transient table's WALs - these are generated because of concurrent
changes done while
applying the logical decoded changes on the new transient table, i think
this won't be a problem
until they only will get recycled but if they get archived , they are of no
use instead they
consume more space and time during the archival process.
"Leverage" Idea:
i think we can re-use these transient table's relation files and WALs
during crash recovery,
so that user don't have to re-run the REPACK (concurrently) after server
has recovered,
for this we might need to write a WAL for REPACK (concurrently) to let
startup process
know REPACK (concurrently) occurred which sets a flag, so at the end of
startup process
all the WALs of the transient table are already applied so transient table
perfect now ,
at the end we can do swapping (finish_heap_swap) after checking the flag ,
these are
all my initial thoughts on this idea to reuse the "residue" files of the
transient table.
I could be totally wrong :) Please correct me if I am.
i think we need to update this statement in repack.sgml regarding wal_level
<listitem>
<para>
The <link
linkend="guc-wal-level"><varname>wal_level</varname></link>
configuration parameter is less than <literal>logical</literal>.
</para>
</listitem>
because of this commit POC: enable logical decoding when wal_level =
'replica' without a server restart (67c2097)
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Attachments:
[application/octet-stream] setup.sql (390B, 3-setup.sql)
download
[application/octet-stream] dual_chaos.sql (891B, 4-dual_chaos.sql)
download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
2026-02-25 13:55 ` Re: Adding REPACK [concurrently] Srinath Reddy Sadipiralla <[email protected]>
@ 2026-02-25 19:41 ` Antonin Houska <[email protected]>
2026-03-24 12:13 ` Re: Adding REPACK [concurrently] Jim Jones <[email protected]>
2026-03-26 13:19 ` Re: Adding REPACK [concurrently] Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 2 replies; 8+ messages in thread
From: Antonin Houska @ 2026-02-25 19:41 UTC (permalink / raw)
To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: [email protected]; Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Srinath Reddy Sadipiralla <[email protected]> wrote:
> I did stress testing on v35 patches, where I did concurrency test using
> pgbench with 50 concurrent clients, 4 threads with the below pgbench
> script (dual_chaos.sql) on the following table setup(setup.sql).
> I ran pgbench with 5M rows for 10 minutes and 50M for ~45 minutes
> multiple times. REPACK (concurrently) ran successfully except "once"(see below).
> I created a shadow/clone table to use for checking the correctness after doing
> the concurrency test.I used 4 checks to verify that data is intact and
> REPACK (concurrently) ran successfully.
>
> 1) table file OID(relfilenode) swapped?
> 2) bloat gone? victim relation size should be less than
> shadow relation size.
> 3) using FULL JOIN logic (borrowed from repack.spec, with small change)
> against the shadow table which goes under the same concurrent ops
> done on the victim table , basically doing dual writes (see dual_chaos.sql) to
> verify table data integrity.
> 4) Physical Index Integrity (amcheck) (borrowed from Mihail's tests)
Thanks!
> The concurrency test failed once. I tried to reproduce the below scenario
> but no luck,i think the reason the assert failure happened because
> after speculative insert there might be no spec CONFIRM or ABORT, thoughts?
Perhaps, I'll try. I'm not sure the REPACK decoding worker does anthing
special regarding decoding. If you happen to see the problem again, please try
to preserve the related WAL segments - if this is a bug in PG executor,
pg_waldump might reveal that.
> Crash Test:
> i did crash test using debugger using a breakpoint inside apply_concurrent_changes
> to simulate a crash while concurrent changes are being done, after few concurrent changes
> are done , i crashed the server using "pg_ctl -m immediate stop", then restarted the server,
> i observed that REPACK (concurrently) didn't completed (expected), files were not swapped and data
> on the victim table is intact checked using FULL JOIN with shadow table, but there are
> some leftovers of the transient table we used for REPACK (concurrently) such as
> 1) transient table's relation files - these consume extra space , i think this was the
> case with VACUUM FULL previously, so these has to be removed manually , but
> I think this time we have a "leverage" which we can use to remove the extra space.
> 2) transient table's WALs - these are generated because of concurrent changes done while
> applying the logical decoded changes on the new transient table, i think this won't be a problem
> until they only will get recycled but if they get archived , they are of no use instead they
> consume more space and time during the archival process.
VACUUM FULL / CLUSTER also produces (a lot of) WAL, so IMO there's nothing
specific about REPACK.
Regarding the transient table, I have a draft patch (for future versions) that
creates the transient table in a separate transaction and commits it. (This is
part of the effort to not block the progress of VACUUM xmin horizon. The point
is that most of the time REPACK should not have XID assigned.) With this
design, each time REPACK starts, it checks (in the pg_depend catalog) if the
transient table exists for particular table, and if it does, it drops it.
> "Leverage" Idea:
> i think we can re-use these transient table's relation files and WALs during crash recovery,
> so that user don't have to re-run the REPACK (concurrently) after server has recovered,
> for this we might need to write a WAL for REPACK (concurrently) to let startup process
> know REPACK (concurrently) occurred which sets a flag, so at the end of startup process
> all the WALs of the transient table are already applied so transient table perfect now ,
> at the end we can do swapping (finish_heap_swap) after checking the flag , these are
> all my initial thoughts on this idea to reuse the "residue" files of the transient table.
> I could be totally wrong :) Please correct me if I am.
I think it'd be quite difficult to restart REPACK exactly at the point it
crashed. Especially if the tables are unlocked between server restart and the
restart of REPACK.
> i think we need to update this statement in repack.sgml regarding wal_level
> <listitem>
> <para>
> The <link linkend="guc-wal-level"><varname>wal_level</varname></link>
> configuration parameter is less than <literal>logical</literal>.
> </para>
> </listitem>
> because of this commit POC: enable logical decoding when wal_level = 'replica' without a server restart (67c2097)
I'm aware of this commit and already updated regression tests, however forgot
to update the user documentation. Thanks for reminder.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
2026-02-25 13:55 ` Re: Adding REPACK [concurrently] Srinath Reddy Sadipiralla <[email protected]>
2026-02-25 19:41 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
@ 2026-03-24 12:13 ` Jim Jones <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Jim Jones @ 2026-03-24 12:13 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: Srinath Reddy Sadipiralla <[email protected]>; [email protected]; Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
On 24/03/2026 09:48, Antonin Houska wrote:
> Thanks for the report. I agree that there's no reason for REPACK to get
> blocked on a lock of a table that it will not process anyway. However, as both
> VACUUM FULL and CLUSTER in PG 18 appear to have the same problem, I'm not sure
> your patch needs to be incorporated in the new feature. Adding a bug fix entry
> to the CF seems the appropriate action to me.
Indeed. VACUUM FULL seems to have the same problem.
I'll include it in the patch and open a new thread.
Best, Jim
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Adding REPACK [concurrently]
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
2026-02-25 13:55 ` Re: Adding REPACK [concurrently] Srinath Reddy Sadipiralla <[email protected]>
2026-02-25 19:41 ` Re: Adding REPACK [concurrently] Antonin Houska <[email protected]>
@ 2026-03-26 13:19 ` Srinath Reddy Sadipiralla <[email protected]>
1 sibling, 0 replies; 8+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-03-26 13:19 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: [email protected]; Mihail Nikalayeu <[email protected]>; Pg Hackers <[email protected]>; Robert Treat <[email protected]>
Hi Antonin,
On Mon, Mar 23, 2026 at 5:30 PM Antonin Houska <[email protected]> wrote:
>
> I could not resist digging in it deeper :-) Attached is a test that
> reproduces
> the crash - it includes the isolation tester enhancement that I posted
> separately [1]. It crashes reliably with v43 [2] if your fix v43-0005 is
> omitted.
>
+1, i tested the same and yeah it reproduces the crash ,also reviewed
the patch LGTM, except i think you need to add this to makefile, so that a
simple
make check would be enough to run the isolation test, i think this all
makes sense
if this [1] patch goes in.
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index acbcaed2feb..201e3c84cb4 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -9,7 +9,10 @@ REGRESS = ddl xact rewrite toast permissions
decoding_in_xact \
ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
oldest_xmin snapshot_transfer subxact_without_top concurrent_stream
\
twophase_snapshot slot_creation_error catalog_change_snapshot \
- skip_snapshot_restore invalidation_distribution
parallel_session_origin
+ skip_snapshot_restore invalidation_distribution
parallel_session_origin \
+ filtering
+
+EXTRA_INSTALL = src/test/modules/injection_points
REGRESS_OPTS = --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf
ISOLATION_OPTS = --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf
[1] - https://www.postgresql.org/message-id/flat/4703.1774250534%40localhost
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2026-03-26 13:19 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-24 18:29 Re: Adding REPACK [concurrently] Alvaro Herrera <[email protected]>
2026-02-25 08:55 ` Antonin Houska <[email protected]>
2026-02-25 13:54 ` Alvaro Herrera <[email protected]>
2026-02-25 16:03 ` Antonin Houska <[email protected]>
2026-02-25 13:55 ` Srinath Reddy Sadipiralla <[email protected]>
2026-02-25 19:41 ` Antonin Houska <[email protected]>
2026-03-24 12:13 ` Jim Jones <[email protected]>
2026-03-26 13:19 ` Srinath Reddy Sadipiralla <[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