From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nathan Bossart <nathandbossart@gmail.com>
cc: pgsql-hackers@lists.postgresql.org, alex work <alexwork033@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>
Subject: Re: Slow GRANT ROLE on PostgreSQL 16 with thousands of ROLEs
In-reply-to: <20240322163952.GA2347986@nathanxps13>
References: <CAGvXd3OSMbJQwOSc-Tq-Ro1CAz=vggErdSG7pv2s6vmmTOLJSg@mail.gmail.com> <341186.1711037256@sss.pgh.pa.us> <619340.1711050134@sss.pgh.pa.us> <642442.1711053105@sss.pgh.pa.us> <20240321204012.GA1992418@nathanxps13> <20240322005139.GA2060816@nathanxps13> <806361.1711069194@sss.pgh.pa.us> <20240322144739.GA2321246@nathanxps13> <20240322145532.GA2346110@nathanxps13> <907785.1711121266@sss.pgh.pa.us> <20240322163952.GA2347986@nathanxps13>
Comments: In-reply-to Nathan Bossart <nathandbossart@gmail.com>
	message dated "Fri, 22 Mar 2024 11:39:52 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <939944.1711126395.1@sss.pgh.pa.us>
Date: Fri, 22 Mar 2024 12:53:15 -0400
Message-ID: <939945.1711126395@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/939945.1711126395%40sss.pgh.pa.us>
Precedence: bulk

Nathan Bossart <nathandbossart@gmail.com> writes:
> On Fri, Mar 22, 2024 at 11:27:46AM -0400, Tom Lane wrote:
>> * Do we want to risk back-patching any of this, to fix the performance
>> regression in v16?  I think that the OP's situation is a pretty
>> narrow one, but maybe he's not the only person who managed to dodge
>> roles_is_member_of's performance issues in most other cases.

> I've heard complaints about performance with many roles before, so I
> certainly think this area is worth optimizing.  As far as back-patching
> goes, my current feeling is that the hash table is probably pretty safe and
> provides the majority of the benefit, but anything fancier should probably
> be reserved for v17 or v18.

Yeah.  Although both the catcache and list_append_unique_oid bits
are O(N^2), the catcache seems to have a much bigger constant
factor --- when I did a "perf" check on the unpatched code,
I saw catcache eating over 90% of the runtime and list_member_oid
about 2%.  So let's fix that part in v16 and call it a day.
It should be safe to back-patch the catcache changes as long as
we put the new fields at the end of the struct and leave cc_lists
present but empty.

Would you like to review the catcache patch further, or do you
think it's good to go?

			regards, tom lane