From: Hamid Akhtar <hamid.akhtar@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Fujii Masao <masao.fujii@gmail.com>
Subject: Re: track_planning causing performance regression
Date: Wed, 19 Aug 2020 12:45:41 +0000
Message-ID: <159784114193.7095.7438157679688812860.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <CANugjhvePOw_fUG0_MF+eGLJ4=8OAWCsW0Wmp2-sTZyq6+3h=Q@mail.gmail.com>
References: <2895b53b033c47ccb22972b589050dd9@EX13D05UWC001.ant.amazon.com>
<CANugjhvePOw_fUG0_MF+eGLJ4=8OAWCsW0Wmp2-sTZyq6+3h=Q@mail.gmail.com>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Overall, the patch works fine. However, I have a few observations:
(1) Code Comments:
- The code comments should be added for the 2 new macros, in particular for PGSS_NUM_LOCK_PARTITIONS. As you explained in your email, this may be used to limit the number of locks if a very large value for pgss_max is specified.
- From the code I inferred that the number of locks can in future be less than pgss_max (per your email where in future this macro could be used to limit the number of locks). I suggest to perhaps add some notes helping future changes in this code area.
(2) It seems like that "pgss->lock = &(pgss->base + pgss_max)->lock;" statement should not use pgss_max directly and instead use PGSS_NUM_LOCK_PARTITIONS macro, as when a limit is imposed on number of locks, this statement will cause an overrun.
--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus
The new status of this patch is: Waiting on Author
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-hackers@postgresql.org
Cc: hamid.akhtar@gmail.com, pgsql-hackers@lists.postgresql.org, masao.fujii@gmail.com
Subject: Re: track_planning causing performance regression
In-Reply-To: <159784114193.7095.7438157679688812860.pgcf@coridan.postgresql.org>
* 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