From: Michael Paquier <michael@paquier.xyz>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: wangsh.fnst@fujitsu.com, osumi.takamichi@fujitsu.com, pgsql-bugs@lists.postgresql.org, Andres Freund <andres@anarazel.de>
Subject: Re: "unexpected duplicate for tablespace" problem in logical replication
Date: Thu, 21 Aug 2025 15:12:18 +0900
Message-ID: <aKa4wnxX6h2siDIq@paquier.xyz> (raw)
In-Reply-To: <CAExHW5uaSBDktUEyohUYy_QGRZgN=zd=B9gdp2MyDMJZOCisBg@mail.gmail.com>
References: <20240112.173746.1710988914918513917.horikyota.ntt@gmail.com>
<CA+TgmoYuONjrkcSDJC60RoaVQ_q3JP5H4b-5uiK+tanONYJYbA@mail.gmail.com>
<20240116.105804.1924548897701268962.horikyota.ntt@gmail.com>
<CA+TgmoYADF0ddp4JO_a8AiUDKCLFGTAw7ii8OniLBzmwVG8njg@mail.gmail.com>
<Zffgupps4Gv9BVXL@paquier.xyz>
<CAD21AoAcGtSOxp-6b7JWVNsyuefk2QwLN2aF2Ki+1bq+X=6fzA@mail.gmail.com>
<CALDaNm2b3XtaA6GQCD55z-eOyxXR2s_oBifesb7FgpKLJNYR0A@mail.gmail.com>
<CAExHW5vaHu=s6eopJ2jxyGNRG--uEsJwYk92yxa2L_80+Y=1YA@mail.gmail.com>
<CAExHW5vBePcL9iEFasECB-wmYksZ9nUVnjnzOqkHYvtbCn==0A@mail.gmail.com>
<CAExHW5uaSBDktUEyohUYy_QGRZgN=zd=B9gdp2MyDMJZOCisBg@mail.gmail.com>
On Wed, Aug 13, 2025 at 03:29:50PM +0530, Ashutosh Bapat wrote:
Sorry for the delay. This has been on my TODO list for some time.
Back to it now.
> On Wed, Aug 13, 2025 at 3:18 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>> On Thu, Jun 19, 2025 at 2:17 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
>>> The description of UNLOGGED mentions "permanent" so it looks like your
>>> use of "permanent" covers both regular and unlogged tables. However
>>> looking purely at the RELPERSISTENCE_ names, it' s possible that one
>>> will associate the word "permanent" with RELPERSISTENCE_PERMANENT
>>> only, and not with RELPERSISTENCE_UNLOGGED. Should we use
>>> "non-temporary" instead to avoid confusion?
FWIW, I see a pretty good point in backpatching what we have here, as
active users of logical replication can be impacted by this issue
randomly. Even if this requires a wraparound, for some users it may
be a window that can open in a couple of days, mostly for workloads
with a high temp table exhaustion, I guess. In short, let's not touch
pg_proc.dat, and I'm planning for a backpatch all the way down due to
the possible random intrusions with the logirep paths.
The changes at the top of RelidByRelfilenumber() describing the
problem with temporary tables feels a bit bloated. I see no need to
mention directly autoprewarm and logical replication, two of the three
callers of this routine, with the explanation coming down to the fact
that this is only a temp relation problem because we cannot pinpoint
to an OID without knowing a proc number. So let's cut a few lines
here at the top of RelidByRelfilenumber(), let's add a simpler line in
func-admin.sgml to say directly and only that "temporary relations are
not supported", passing on the "permanent" vs "no permanent" business.
I don't see a direct need to mention that at the top of
pg_relation_filenode(), as long as RelidByRelfilenumber(), but OK by
me to add a simpler "temporary relations are not supported", it looks
like most prefer than based on the contents of the thread.
>>> Logical replication and autoprewarm may not cause such a large bloat
>>> but there is no limit to passing invalid combinations of reltablespace
>>> and relfilenumber to pg_filenode_relation(). Do we want to prohibit
>>> that case by passing a flag from logical pg_filenode_relation() to not
>>> cache invalid entries?
Fun. Yes, I agree that we could do better here. It does not strike
me as an issue as invasive as the original report, direct calls of
pg_filenode_relation() are much rarer than the code paths of
autoprewarm and logical replication touched by RelidByRelfilenumber().
That's a potential HEAD improvement to me.
> Please ignore the 0002 patch in the earlier patchset. It's an
> experimental change related to negative entries bloat in
> RelfilenumberMap cache.
And I was wondering what was going on in the latest patch set.. I'm
ignoring this part.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../aKa4wnxX6h2siDIq@paquier.xyz/2-signature.asc)
download
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-bugs@postgresql.org
Cc: michael@paquier.xyz, ashutosh.bapat.oss@gmail.com, vignesh21@gmail.com, sawada.mshk@gmail.com, robertmhaas@gmail.com, horikyota.ntt@gmail.com, andres@anarazel.de
Subject: Re: "unexpected duplicate for tablespace" problem in logical replication
In-Reply-To: <aKa4wnxX6h2siDIq@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox