Date: Mon, 6 Apr 2026 18:10:56 -0700
From: Noah Misch <noah@leadboat.com>
To: Andres Freund <andres@anarazel.de>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
	vignesh C <vignesh21@gmail.com>, Antonin Houska <ah@cybertec.at>,
	Srinath Reddy Sadipiralla <srinath2133@gmail.com>,
	Amit Kapila <amit.kapila16@gmail.com>,
	Mihail Nikalayeu <mihailnikalayeu@gmail.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>,
	Robert Treat <rob@xzilla.net>
Subject: Re: Adding REPACK [concurrently]
Message-ID: <20260407011056.50.noahmisch@microsoft.com>
References: 
 <CALDaNm3tiKhtegx5Cawi34UjbHmNGEDNAtScGM1RgWRtV-5_0Q@mail.gmail.com>
 <202604060918.qw5ms7cbr2hz@alvherre.pgsql>
 <pbqm52nsviwojptiszg2g6pqntbhnhaotbxshiysnensshnuv5@v6pmney4jror>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <pbqm52nsviwojptiszg2g6pqntbhnhaotbxshiysnensshnuv5@v6pmney4jror>
User-Agent: Mutt/2.3.0 (2026-01-25)
Archived-At: 
 <https://www.postgresql.org/message-id/20260407011056.50.noahmisch%40microsoft.com>
Precedence: bulk

On Mon, Apr 06, 2026 at 05:11:30PM -0400, Andres Freund wrote:
>   heap_insert()
>   ->CacheInvalidateHeapTuple()
>   ->CacheInvalidateHeapTupleCommon()
>   ->AssertCouldGetRelation()
> not being cheap and running a *lot*.
> 
> Admittedly it's way worse if you build with -O0, which I tend to do to make
> debugging easier.
> 
> In that config, the assert single-handled increases the time for a repack by
> 35% or so.
> 
> 
> Noah, is there any reason we need to do the AssertCouldGetRelation() before
> the !IsCatalogRelation(relation)? Given that the goal is to make
> RelationGetRelid() safe, it doesn't seem there is?

By running AssertCouldGetRelation() during every INSERT statement, this
detects cases that would be unsafe when the target of the INSERT happens to be
a system catalog.  Little of our INSERT/UPDATE coverage targets a system
catalog.  Hence, the current position is better for detection.

I wonder if this got slower in v19.  In v14-v18, the assert's cost is
proportional to the number of held lwlocks, often 0 or 1.  In v19, it's
proportional to PrivateRefCountHash cardinality.