public inbox for [email protected]  
help / color / mirror / Atom feed
From: Antonin Houska <[email protected]>
To: Mihail Nikalayeu <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: Robert Treat <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Wed, 27 Aug 2025 12:11:45 +0200
Message-ID: <17670.1756289505@localhost> (raw)
In-Reply-To: <CADzfLwUYc+9X44r=kf8RmivD_z4xs0MyoZBiE-TsBZrppq64sA@mail.gmail.com>
References: <CADzfLwXx46j8KwQjjM1ZcqNBsx-k6GxHOzDJkm4SHjh+cv31Rw@mail.gmail.com>
	<[email protected]>
	<CADzfLwW=b=U3e6aasi=XorN8hZSiCKZErKs9qhyK7m=w=wokAg@mail.gmail.com>
	<40729.1755799624@localhost>
	<CADzfLwXpyzGGVB+nbSAMBWDBhzTyn6F2hRrAqcyJd3c6gT2EOQ@mail.gmail.com>
	<9536.1756127358@localhost>
	<CADzfLwWdw=5KhXE5Fr3khytp4mW2girFVJHHb4jSgEncc6237Q@mail.gmail.com>
	<21931.1756136535@localhost>
	<CADzfLwWPihwPM=e=EJ_U5niqDdVyq9pS30ZNxwZ8f_yXyikooQ@mail.gmail.com>
	<24483.1756142534@localhost>
	<CADzfLwUqyOmpkLmciecBy4aBN1sohQVZ2Hgc6m-tjSUqDRHwyQ@mail.gmail.com>
	<4790.1756197960@localhost>
	<CADzfLwV+80MfPM=MC5y3nA34djUWuYU6YKcZUO8JjD7_8p7nkg@mail.gmail.com>
	<29527.1756215093@localhost>
	<CADzfLwW3XAFKYkBY7Jktf_rETzcqMj20GXgjESqu+WEoeEMkog@mail.gmail.com>
	<6931.1756275367@localhost>
	<CADzfLwUYc+9X44r=kf8RmivD_z4xs0MyoZBiE-TsBZrppq64sA@mail.gmail.com>

Mihail Nikalayeu <[email protected]> wrote:

> > A new kind of snapshot seems like (much) cleaner solution at the moment.
> 
> Do you mean some kind of snapshot which only uses
> TransactionIdDidCommit/Abort ignoring
> TransactionIdIsCurrentTransactionId/TransactionIdIsInProgress?
> Actually it behaves like SnapshotBelieveEverythingCommitted in that
> particular case, but TransactionIdDidCommit/Abort may be used as some
> kind of assert/error source to be sure everything is going as
> designed.

Given that there should be no (sub)transaction aborts in the new table, I
think you only need to check that the tuple has valid xmin and invalid xmax.

I think the XID should be in the commit log at the time the transaction is
being replayed, so it should be legal to use TransactionIdDidCommit/Abort in
Assert() statements. (And as long as REPACK CONCURRENTLY will use
ShareUpdateExclusiveLock, which conflicts with VACUUM, pg_class(relfrozenxid)
for given table should not advance during the processing, and therefore the
replayed XIDs should not be truncated from the commit log while REPACK
CONCURRENTLY is running.)

> And, yes, for the new snapshot we need to have
> HeapTupleSatisfiesUpdate to be modified.
> 
> Also, to deal with that particular race we may just use
> XactLockTableWait(xid, NULL, NULL, XLTW_None) before starting
> transaction replay.

Do you mean the race related to TransactionIdIsInProgress()? Not sure I
understand, as you suggested above that you no longer need the function.

> > No rush. First, the MVCC safety is not likely to be included in v19 [1].
> 
> That worries me - it is not the behaviour someone expects from a
> database by default. At least the warning should be much more visible
> and obvious.
> I think most of user will expect the same guarantees as [CREATE|RE]
> INDEX CONCURRENTLY provides.

It does not really worry me. The pg_squeeze extension is not MVCC-safe and I
remember there were only 1 or 2 related complaints throughout its
existence. (pg_repack isn't MVCC-safe as well, but I don't keep track of its
issues.)

Of course, user documentation should warn about the problem, in a way it does
for other commands (typically ALTER TABLE).

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





view thread (106+ 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], [email protected]
  Subject: Re: Adding REPACK [concurrently]
  In-Reply-To: <17670.1756289505@localhost>

* 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