From: Antonin Houska <ah@cybertec.at>
To: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
    Fujii Masao <masao.fujii@gmail.com>, Robert Treat <rob@xzilla.net>,
    Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Adding REPACK [concurrently]
In-reply-to: 
 <CADzfLwXpyzGGVB+nbSAMBWDBhzTyn6F2hRrAqcyJd3c6gT2EOQ@mail.gmail.com>
References: 
 <CADzfLwXx46j8KwQjjM1ZcqNBsx-k6GxHOzDJkm4SHjh+cv31Rw@mail.gmail.com>
 <202508091333.qvgvo7ikuezm@alvherre.pgsql>
 <CADzfLwW=b=U3e6aasi=XorN8hZSiCKZErKs9qhyK7m=w=wokAg@mail.gmail.com>
 <40729.1755799624@localhost>
 <CADzfLwXpyzGGVB+nbSAMBWDBhzTyn6F2hRrAqcyJd3c6gT2EOQ@mail.gmail.com>
Comments: In-reply-to Mihail Nikalayeu <mihailnikalayeu@gmail.com>
   message dated "Sun, 24 Aug 2025 18:52:00 +0200."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Mon, 25 Aug 2025 15:09:18 +0200
Message-ID: <9536.1756127358@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/9536.1756127358%40localhost>
Precedence: bulk

Mihail Nikalayeu <mihailnikalayeu@gmail.com> wrote:

> I was looking into catalog-related logical decoding features, and it
> seems like they are clearly overkill for the repack case.
> We don't need CID tracking or even a snapshot for each commit if we=E2=80=
=99re
> okay with passing xmin/xmax as arguments.

I assume you are concerned with the patch part 0005 of the v12 patch
("Preserve visibility information of the concurrent data changes."), aren't
you?

> What do you think about the following approach for replaying:
> * use the extracted XID as the value for xmin/xmax.
> * use SnapshotSelf to find the tuple for update/delete operations.
>=20
> SnapshotSelf seems like a good fit here:
> * it sees the last "existing" version.
> * any XID set as xmin/xmax in the repacked version is already
> committed - so each update/insert is effectively "committed" once
> written.
> * it works with multiple updates of the same tuple within a single
> transaction - SnapshotSelf sees the last version.
> * all updates are ordered and replayed sequentially - so the last
> version is always the one we want.
>=20
> If I'm not missing anything, this looks like something worth including
> in the patch set.
> If so, I can try implementing a test version.

Not sure I understand in all details, but I don't think SnapshotSelf is the
correct snapshot. Note that HeapTupleSatisfiesSelf() does not use its
'snapshot' argument at all. Instead, it considers the set of running
transactions as it is at the time the function is called.

One particular problem I imagine is replaying an UPDATE to a row that some
later transaction will eventually delete, but the transaction that ran the
UPDATE obviously had to see it. When looking for the old version during the
replay, HeapTupleSatisfiesMVCC() will find the old version as long as we pa=
ss
the correct snapshot to it.

However, at the time we're replaying the UPDATE in the new table, the tuple
may have been already deleted from the old table, and the deleting transact=
ion
may already have committed. In such a case, HeapTupleSatisfiesSelf() will
conclude the old version invisible and the we'll fail to replay the UPDATE.

--=20
Antonin Houska
Web: https://www.cybertec-postgresql.com