agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Antonin Houska <ah@cybertec.at>
To: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Robert Treat <rob@xzilla.net>
Cc: Fujii Masao <masao.fujii@gmail.com>
Subject: Re: Adding REPACK [concurrently]
Date: Mon, 01 Sep 2025 17:30:18 +0200
Message-ID: <51329.1756740618@localhost> (raw)
In-Reply-To: <CADzfLwU4kuHSPd9ty8DQpRNWRjX6rJJVVzzWvT4+MEoTyyGtDg@mail.gmail.com>
References: <202508301750.cbohxyy2pcce@alvherre.pgsql>
	<202508311209.gnvpor56qkdc@alvherre.pgsql>
	<CADzfLwWJqoG6uPt+HywKOFjXhqSbfCr+VXpfio9YQ6yqQaihPA@mail.gmail.com>
	<4607.1756703531@localhost>
	<CADzfLwU4kuHSPd9ty8DQpRNWRjX6rJJVVzzWvT4+MEoTyyGtDg@mail.gmail.com>

Mihail Nikalayeu <mihailnikalayeu@gmail.com> wrote:

> Antonin Houska <ah@cybertec.at>:
> > Are you sure the test is complete? I see no occurrence of the REPACK command
> > in it.
> Oops, send invalid file. The correct one in attachment.

Thanks!

The problem was that when removing the original "preserve visibility patch"
v12-0005 [1] from the series, I forgot to change the value of
'need_full_snapshot' argument of CreateInitDecodingContext().

v12 and earlier treated the repacked table like system catalog, so it was
o.k. to pass need_full_snapshot=false. However, it must be true now, otherwise
the snapshot created for the initial copy does not see commits of transactions
that do not change regular catalogs.

The fix is as simple as

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index f481a3cec6d..7866ac01278 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -502,6 +502,7 @@ SnapBuildInitialSnapshotForRepack(SnapBuild *builder)
        StringInfo      buf = makeStringInfo();
 
        Assert(builder->state == SNAPBUILD_CONSISTENT);
+       Assert(builder->building_full_snapshot);
 
        snap = SnapBuildBuildSnapshot(builder);
 

I'll apply it to the next version of the "Add CONCURRENTLY option to REPACK
command" patch.


[1] https://www.postgresql.org/message-id/flat/CAFj8pRDK89FtY_yyGw7-MW-zTaHOCY4m6qfLRittdoPocz+dMQ@mail....

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





view thread (416+ messages)  latest in thread

Message-ID: <51329.1756740618@localhost>
Permalink:  ../51329.1756740618@localhost/
Also on:    postgresql.org/message-id/51329.1756740618@localhost

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: pgsql-hackers@postgresql.org
  Cc: ah@cybertec.at, mihailnikalayeu@gmail.com, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org, rob@xzilla.net, masao.fujii@gmail.com
  Subject: Re: Adding REPACK [concurrently]
  In-Reply-To: <51329.1756740618@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