Hi Sirisha,
Thanks for identifying the bug and the solution. Some review comments inlined.
On Mon, Nov 21, 2022 at 2:49 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2022-Nov-20, sirisha chamarthi wrote:
>
> > Hi Hackers,
> >
> > forking this thread from the discussion [1] as suggested by Amit.
> >
> > Catalog_xmin is not advanced when a logical slot is invalidated (lost)
> > until the invalidated slot is dropped. This patch ignores invalidated slots
> > while computing the oldest xmin. Attached a small patch to address this and
> > the output after the patch is as shown below.
>
> Oh wow, that's bad :-( I'll get it patched immediately.
+ /* ignore invalid slots while computing the oldest xmin */
+ if (TransactionIdIsValid(invalidated_at_lsn))
+ continue;
I think the condition should be
if (!XLogRecPtrIsInvalid(invalidated_at_lsn)) LSN and XID are
different data types.
and to be inline with pg_get_replication_slots()
361 if (XLogRecPtrIsInvalid(slot_contents.data.restart_lsn) &&
362 !XLogRecPtrIsInvalid(slot_contents.data.invalidated_at))
363 walstate = WALAVAIL_REMOVED;
we should also check restart_lsn.
I would write this as
bool invalidated_slot = false;
then under spinlock
invalidated_slot = XLogRecPtrIsInvalid(s->data.restart_lsn) &&
!XLogRecPtrIsInvalid(s->data.invalidated_at);
if (invalidated_slot)
continue.
--
Best Wishes,
Ashutosh Bapat