Horton

Re: Adding REPACK [concurrently]

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Antonin Houska <ah@cybertec.at>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Srinath Reddy Sadipiralla <srinath2133@gmail.com>, Mihail Nikalayeu <mihailnikalayeu@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>, Robert Treat <rob@xzilla.net>
Date: 2026-05-28T00:31:59Z
Lists: pgsql-hackers 
On Wed, May 27, 2026 at 1:08 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> 0001 remains unchanged.
>

Few minor comments:
=================
*
+ * To allow old WAL files to be recycled, we manually advance the
+ * slot each time a WAL segment boundary is crossed.

This is  safe only because REPACK creates a temporary slot that is
dropped if REPACK fails — there's no scenario where this slot needs to
restart decoding from
an earlier position while still alive. I feel that is worth a mention.

*
@@ -1910,8 +1910,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();

This change is not related to this patch, rather we need it even
without this patch, is it worth mentioning in the commit message?

-- 
With Regards,
Amit Kapila.