MIME-Version: 1.0
References: <202512151349.vlq3mpfniyk3@alvherre.pgsql> <11247.1767609087@localhost>
 <11558.1767609632@localhost> <141054.1767891540@localhost>
 <CADzfLwU-OmxW3t3AoQo9=K7uq4G1yZ-txcetzW3jbcVxV_pJew@mail.gmail.com>
 <137668.1768235610@localhost> <CADzfLwUJSHKGxYw+vMUZ_Hr2YeuxO2Q5w13HKgUUN1725tjY5Q@mail.gmail.com>
 <CADzfLwXJ+4s1tJuG9injcxAUP3urj9D6dUAPOCaX33UeiUxrRQ@mail.gmail.com> <74802.1769071060@localhost>
In-Reply-To: <74802.1769071060@localhost>
From: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Date: Thu, 22 Jan 2026 12:30:00 +0100
Message-ID: <CADzfLwVZ_DeU_3avD=G4ZHFJJgZ0EOFzxnmWxwyB23zsS-uxjA@mail.gmail.com>
Subject: Re: Adding REPACK [concurrently]
To: Antonin Houska <ah@cybertec.at>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, 
	Pg Hackers <pgsql-hackers@lists.postgresql.org>, Robert Treat <rob@xzilla.net>
Content-Type: multipart/alternative; boundary="0000000000009f238d0648f85ddb"
Archived-At: <https://www.postgresql.org/message-id/CADzfLwVZ_DeU_3avD%3DG4ZHFJJgZ0EOFzxnmWxwyB23zsS-uxjA%40mail.gmail.com>
Precedence: bulk

--0000000000009f238d0648f85ddb
Content-Type: text/plain; charset="UTF-8"

Hello, Antonin!

> The changes present in WAL decoded prior the snapshot creation are not
> replayed - these changes are visible to the snapshot. (This is not really
> specific to the 0006 part.)

OK, just want to be sure it still works the same way if we build multiple
snapshots for the same slot that way.

> The current API does not seem to support changing snapshot of an
in-progress
> scan and I don't want to change that. Plus note that the current
> implementation of CLUSTER also uses SnapshotAny and then checks the
visibility
> separately. Finally, SnapshotAny is not really an expensive visibility
check,
> if it can be considered a visibility check at all.

But we will require a real check for each tuple. Including dead one,
multiple versions of the same HOT, etc.

> I've added it only for xmin. xid is valid because REPACK is executed in a
> transaction. That reminds me that PROC_IN_VACUUM should be present in
> MyProc->statusFlags. Fixed.

Yes, xid is required for repack. I think it is better to introduce a new
flag instead of PROC_IN_VACCUUM.


> > > PushActiveSnapshot(GetTransactionSnapshot());
> > GetLatestSnapshot() feels better here.
> What will then happen to code that uses GetActiveSnapshot() ?

O, I mean PushActiveSnapshot(GetLatestSnapshot())

> > Also, to correctly build a unique index - some tech from [0] is
required (building a unique index with multiple snapshots is a little bit
tricky).
> ok, I'll check your patch.

I realized building a unique index is still done with a single snapshot, so
it should be OK for that case. But still check the patch :)

>  I proposed the Assert above, but still thinking about it.
Hm... Do we really need these asserts if PROC_IN_VACUUM is set? I was
proposing a way it is used for index building (to ensure nothing is
propagated into xmin).

Best regards,
Mikhail.

--0000000000009f238d0648f85ddb
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hello, Antonin!</div><div><br></div><div>&gt; The cha=
nges present in WAL decoded prior the snapshot creation are not<br>&gt; rep=
layed - these changes are visible to the snapshot. (This is not really<br>&=
gt; specific to the 0006 part.)<span class=3D"gmail-im"><br></span></div><d=
iv><span class=3D"gmail-im"><br></span></div><div><span class=3D"gmail-im">=
OK, just want to be sure it still works the same way if we build multiple s=
napshots for the same slot that=C2=A0way.</span></div><div><span class=3D"g=
mail-im"><br></span></div><div>&gt; The current API does not seem to suppor=
t changing snapshot of an in-progress<br>&gt; scan and I don&#39;t want to =
change that. Plus note that the current<br>&gt; implementation of CLUSTER a=
lso uses SnapshotAny and then checks the visibility<br>&gt; separately. Fin=
ally, SnapshotAny is not really an expensive visibility check,<br>&gt; if i=
t can be considered a visibility check at all.</div><div><br><span class=3D=
"gmail-im"></span></div><div><span class=3D"gmail-im">But we will require a=
 real check for each tuple. Including dead one, multiple versions of the sa=
me HOT, etc.</span></div><div><span class=3D"gmail-im"><br></span></div><di=
v>&gt; I&#39;ve added it only for xmin. xid is valid because REPACK is exec=
uted in a<br>&gt; transaction. That reminds me that PROC_IN_VACUUM should b=
e present in<br>&gt; MyProc-&gt;statusFlags. Fixed.<span class=3D"gmail-im"=
><br></span></div><div><span class=3D"gmail-im"><br></span></div><div>Yes, =
xid is required for repack. I think it is better to introduce a new flag in=
stead of PROC_IN_VACCUUM.</div><div><br></div><div><br></div><div><span cla=
ss=3D"gmail-im">&gt; &gt; &gt; PushActiveSnapshot(GetTransactionSnapshot())=
;<br>&gt; &gt; GetLatestSnapshot() feels better here.<br></span>&gt; What w=
ill then happen to code that uses GetActiveSnapshot() ?<span class=3D"gmail=
-im"><br></span></div><div><span class=3D"gmail-im"><br></span></div><div><=
span class=3D"gmail-im">O, I mean=C2=A0</span><span class=3D"gmail-im">Push=
ActiveSnapshot(</span><span class=3D"gmail-im">GetLatestSnapshot</span><spa=
n class=3D"gmail-im">())</span></div><div><br></div><div><span class=3D"gma=
il-im">&gt; &gt; Also, to correctly build a unique index - some tech from [=
0] is=20
required (building a unique index with multiple snapshots is a little=20
bit tricky).<br></span>&gt; ok, I&#39;ll check your patch.<span class=3D"gm=
ail-im"><br></span></div><div><span class=3D"gmail-im"><br></span></div><di=
v><span class=3D"gmail-im">I realized building a unique index is still done=
 with a single snapshot, so it should be OK for that case. But still check =
the patch :)</span></div><div><span class=3D"gmail-im"><br></span></div><di=
v>&gt;=C2=A0<span class=3D"gmail-im">
</span>I proposed the Assert above, but still thinking about it.<span class=
=3D"gmail-im"><br></span></div><div><span class=3D"gmail-im">Hm... Do we re=
ally need these asserts if=C2=A0</span>PROC_IN_VACUUM is set? I was proposi=
ng a way it is used for index building (to ensure nothing is propagated int=
o xmin).</div><div><br></div><div>Best regards,</div><div>Mikhail.</div><di=
v><br></div></div>

--0000000000009f238d0648f85ddb--