Re: Don't overwrite scan key in systable_beginscan()

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Don't overwrite scan key in systable_beginscan()
Date: 2024-11-26 13:56:12
Message-ID: Z0XTfIq5xUtbkiIh@pryzbyj2023
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 12, 2024 at 09:44:02AM +0200, Peter Eisentraut wrote:
> Second (or 0005), an alternative to palloc is to make the converted scan
> keys a normal local variable. Then it's just a question of whether a
> smaller palloc is preferred over an over-allocated local variable. I think
> I still prefer the palloc version, but it doesn't matter much either way I
> think.

Since 811af9786b, the palloc'd idxkey's seem to be leaking/accumulating
throughout the command.

I noticed this on the master branch while running ANALYZE on partitioned
table with 600 attributes, even though only 6 were being analyzed.

LOG: level: 3; BuildRelationExtStatistics: 1239963512 total in 278 blocks; 5082984 free (296 chunks); 1234880528 used

Several indexes are being scanned many thousands of times.

310690 beginscan 2659
310689 beginscan 2662
77672 beginscan 2678

postgres=# SELECT 2659::REGCLASS;
regclass | pg_attribute_relid_attnum_index

postgres=# SELECT 2662::REGCLASS;
regclass | pg_class_oid_index

postgres=# SELECT 2678::REGCLASS;
regclass | pg_index_indrelid_index

> From 2f84e4d21ce611a4e6f428f72a18518e22776400 Mon Sep 17 00:00:00 2001
> From: Peter Eisentraut <peter(at)eisentraut(dot)org>
> Date: Thu, 8 Aug 2024 08:27:26 +0200
> Subject: [PATCH v2 1/5] Don't overwrite scan key in systable_beginscan()
...
> Fix that by making a copy of the scan keys passed by the caller and
> make the modifications there.
>
> diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
> index de751e8e4a3..b5eba549d3e 100644
> --- a/src/backend/access/index/genam.c
> +++ b/src/backend/access/index/genam.c
> @@ -419,17 +419,22 @@ systable_beginscan(Relation heapRelation,
> if (irel)
> {
> int i;
> + ScanKey idxkey;
>
> - /* Change attribute numbers to be index column numbers. */
> + idxkey = palloc_array(ScanKeyData, nkeys);
> +
> + /* Convert attribute numbers to be index column numbers. */
> for (i = 0; i < nkeys; i++)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melih Mutlu 2024-11-26 14:11:16 Re: Separate memory contexts for relcache and catcache
Previous Message Jacob Champion 2024-11-26 13:51:20 Re: [PoC] Federated Authn/z with OAUTHBEARER