public inbox for [email protected]  
help / color / mirror / Atom feed
From: Zhijie Hou (Fujitsu) <[email protected]>
To: Antonin Houska <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Srinath Reddy Sadipiralla <[email protected]>
Cc: Mihail Nikalayeu <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Robert Treat <[email protected]>
Subject: RE: Adding REPACK [concurrently]
Date: Mon, 25 May 2026 06:26:52 +0000
Message-ID: <TY4PR01MB17718B44164522D0798F8E898940A2@TY4PR01MB17718.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <125085.1775827305@localhost>
References: <CAA4eK1Jwguq90CC8nxOqan+raCS8WisB=Bmb1AmK8rcvtj8GPg@mail.gmail.com>
	<[email protected]>
	<TYRPR01MB14195633567DA00ABD42570B794592@TYRPR01MB14195.jpnprd01.prod.outlook.com>
	<125085.1775827305@localhost>



> -----Original Message-----
> From: Antonin Houska <[email protected]>
> Sent: Friday, April 10, 2026 9:22 PM
> To: Hou, Zhijie/侯 志杰 <[email protected]>
> Cc: Alvaro Herrera <[email protected]>; Amit Kapila
> <[email protected]>; Kuroda, Hayato/黒田 隼人
> <[email protected]>; Srinath Reddy Sadipiralla
> <[email protected]>; Mihail Nikalayeu <[email protected]>;
> Matthias van de Meent <[email protected]>; Pg Hackers
> <[email protected]>; Robert Treat <[email protected]>
> Subject: Re: Adding REPACK [concurrently]
> 
> Zhijie Hou (Fujitsu) <[email protected]> wrote:
> 
> > When testing REPACK concurrently, I noticed that all WALs are retained
> > from the moment REPACK begins copying data to the new table until the
> > command finishes replaying concurrent changes on the new table and
> > stops the repack decoding worker.
> >
> > I understand the reason: the REPACK command itself starts a
> > long-running transaction, and logical decoding does not advance
> > restart_lsn beyond the oldest running transaction's start position. As
> > a result, slot.restart_lsn remains unchanged, preventing the checkpointer
> from recycling WALs.
> 
> I think you're right, sorry for the omission.
> 
> > IIUC, REPACK without using concurrent option does not have this issue.
> 
> It does not have the WAL recycling issue because it does not need to read
> WAL. However it also runs in a long transaction. Even though it does not need
> XID for the actual heap rewriting, it gets one at the moment it locks the table
> using AccessExclusiveLock (which is at the very beginning).
> 
> > Given that we do not restart a REPACK, I think the repack decoding
> > worker should be able to advance restart_lsn each time after writing
> > changes (similar to how a physical slot behaves). To illustrate this,
> > I've written a patch (attached) that implements this approach, and it works
> fine for me.
> 
> LGTM, thanks!
> 

Thanks for reviewing!

After listening to the REPACK talk at pgconf.dev this year, I understand that
WAL accumulation during REPACK CONCURRENTLY is not intended behavior. I think we
can consider fixing this in the current release. Attached is the rebased
patch, with comments adjusted based on Chao Li's comments.

Best Regards,
Hou zj


Attachments:

  [application/octet-stream] v2-0001-Allow-old-WAL-recycling-during-REPACK-CONCURRENTL.patch (3.0K, 2-v2-0001-Allow-old-WAL-recycling-during-REPACK-CONCURRENTL.patch)
  download | inline diff:
From 87ba918b9020ce9fa0d4852f222221f521d8701f Mon Sep 17 00:00:00 2001
From: Zhijie Hou <[email protected]>
Date: Fri, 10 Apr 2026 16:24:55 +0800
Subject: [PATCH v2] Allow old WAL recycling during REPACK CONCURRENTLY

During REPACK CONCURRENTLY, logical decoding can keep replication
slot.restart_lsn pinned behind the oldest running transaction, which is often
the long-lived REPACK transaction itself. As a result, old WAL segments are
retained longer than necessary.

This commit advances the replication slot each time WAL insertion crosses a
segment boundary, so obsolete WAL files can be recycled while REPACK is still
running.

This change does not advance catalog_xmin. REPACK already holds a snapshot that
prevents catalog dead tuple removal, so catalog_xmin handling can be addressed
independently.
---
 src/backend/commands/repack_worker.c      | 14 +++++++++++++-
 src/backend/replication/logical/logical.c |  8 +++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index b84041372b8..eed69d36508 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -397,12 +397,24 @@ decode_concurrent_changes(LogicalDecodingContext *ctx,
 
 			/*
 			 * If WAL segment boundary has been crossed, inform the decoding
-			 * system that the catalog_xmin can advance.
+			 * system that the slot can advance.
+			 *
+			 * Once REPACK begins copying data to the new table, the logical
+			 * decoding machinery prevents the slot from advancing beyond the
+			 * oldest running transaction (which is the REPACK transaction
+			 * itself). As a result, restart_lsn and catalog_xmin can no
+			 * longer advance automatically.
+			 *
+			 * To allow old WAL files to be recycled, we manually advance the
+			 * slot each time a WAL segment boundary is crossed. We do not
+			 * advance catalog_xmin here because the REPACK transaction anyway
+			 * holds a snapshot that prevents catalog dead tuple removal.
 			 */
 			end_lsn = ctx->reader->EndRecPtr;
 			XLByteToSeg(end_lsn, segno_new, wal_segment_size);
 			if (segno_new != repack_current_segment)
 			{
+				LogicalIncreaseRestartDecodingForSlot(end_lsn, end_lsn);
 				LogicalConfirmReceivedLocation(end_lsn);
 				elog(DEBUG1, "REPACK: confirmed receive location %X/%X",
 					 (uint32) (end_lsn >> 32), (uint32) end_lsn);
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index a33a685dcc6..11b64de3d90 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -1913,8 +1913,14 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
 			SpinLockRelease(&MyReplicationSlot->mutex);
 
 			ReplicationSlotsComputeRequiredXmin(false);
-			ReplicationSlotsComputeRequiredLSN();
 		}
+
+		/*
+		 * Now the new restart_lsn is safely on disk, recompute the global WAL
+		 * retention requirement.
+		 */
+		if (updated_restart)
+			ReplicationSlotsComputeRequiredLSN();
 	}
 	else
 	{
-- 
2.43.0



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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: Adding REPACK [concurrently]
  In-Reply-To: <TY4PR01MB17718B44164522D0798F8E898940A2@TY4PR01MB17718.jpnprd01.prod.outlook.com>

* 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