public inbox for [email protected]  
help / color / mirror / Atom feed
From: Mihail Nikalayeu <[email protected]>
To: Antonin Houska <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Robert Treat <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Thu, 22 Jan 2026 12:30:00 +0100
Message-ID: <CADzfLwVZ_DeU_3avD=G4ZHFJJgZ0EOFzxnmWxwyB23zsS-uxjA@mail.gmail.com> (raw)
In-Reply-To: <74802.1769071060@localhost>
References: <[email protected]>
	<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>

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.


view thread (31+ messages)  latest in thread

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]
  Subject: Re: Adding REPACK [concurrently]
  In-Reply-To: <CADzfLwVZ_DeU_3avD=G4ZHFJJgZ0EOFzxnmWxwyB23zsS-uxjA@mail.gmail.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