Message-ID: <5a5be673-4102-4c52-b504-1b205d5b7f83@vondra.me>
Date: Mon, 23 Sep 2024 00:50:41 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: scalability bottlenecks with (many) partitions (and more)
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jakub Wartak <jakub.wartak@enterprisedb.com>,
 Robert Haas <robertmhaas@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>
References: <510b887e-c0ce-4a0c-a17a-2c6abb8d9a5c@enterprisedb.com>
 <CA+TgmoabtPvTffKQiqKg9qOmyt-sYzjQW+TyFWf1CBN7M4hggQ@mail.gmail.com>
 <9266c8df-0df7-4605-ba5a-b204c5e52586@enterprisedb.com>
 <52afd566-d110-43ea-ae72-69349d4414f6@vondra.me>
 <cba2406a-66bb-41ac-b1cf-bb898596e0e1@vondra.me>
 <CA+TgmoZ-40G7-WoLSsEH+7FqUzrP_0ndCOkewJ7ifAO6MVx0EA@mail.gmail.com>
 <b8c43eda-0c3f-4cb4-809b-841fa5c40ada@vondra.me>
 <CA+TgmobCBse3LYj9pfh-ELbP0VnffQS7jP_vNTGA7-KZP0gqDw@mail.gmail.com>
 <52f026c4-6c4e-4de2-bd78-609091d7c772@vondra.me>
 <CA+TgmoamAOj5e4J7WcC9g10AskvVPk5D7JoAvWw8DFnpHMiSQA@mail.gmail.com>
 <14cd8763-c905-4705-99c5-cbcc5db06f50@vondra.me>
 <0775ba8a-920c-4d37-b202-989a90b6ee4c@vondra.me>
 <c7282183-6a74-4797-9659-92e948e713b5@vondra.me>
 <CAKZiRmze8L6FeAKNQhZkn4NmUH2WZZyQ46GUm-Xuky5qr5DUoA@mail.gmail.com>
 <0f27b64b-5bf3-4140-98b7-635e312e1796@vondra.me>
 <3cebb4ab-1168-4259-8cb8-8a8ed7efeb43@vondra.me>
 <7c1eeafb-2375-4ff6-8469-0640d52d44ed@vondra.me>
 <2891628.1727019959@sss.pgh.pa.us>
Content-Language: en-US
From: Tomas Vondra <tomas@vondra.me>
In-Reply-To: <2891628.1727019959@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/5a5be673-4102-4c52-b504-1b205d5b7f83%40vondra.me>
Precedence: bulk

On 9/22/24 17:45, Tom Lane wrote:
> Tomas Vondra <tomas@vondra.me> writes:
>> I've finally pushed this, after many rounds of careful testing to ensure
>> no regressions, and polishing.
> 
> Coverity is not terribly happy with this.  "Assert(fpPtr = fpEndPtr);"
> is very clearly not doing what you presumably intended.  The others
> look like overaggressive assertion checking.  If you don't want those
> macros to assume that the argument is unsigned, you could force the
> issue, say with
> 
>  #define FAST_PATH_GROUP(index)	\
> -	(AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
> +	(AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_BACKEND), \
>  	 ((index) / FP_LOCK_SLOTS_PER_GROUP))
> 

Ah, you're right. I'll fix those asserts tomorrow.

The first is clearly wrong, of course.

For the (x >= 0) asserts, doing it this way relies on negative values
wrapping to large positive ones, correct? AFAIK it's guaranteed to be a
very large value, so it can't accidentally be less than the slot count.


regards

-- 
Tomas Vondra