Hi, On Wed, Oct 23, 2024 at 01:19:37AM +0900, Fujii Masao wrote: > > I understand that PGPROC entries with pid=0 are typically those not yet > allocated to > any backends. Yes, as you mentioned, prepared transactions also have pid=0. > However, > GetLockStatusData() loops up to ProcGlobal->allProcCount, which is > MaxBackends plus > NUM_AUXILIARY_PROCS, excluding prepared transactions. Therefore, > GetLockStatusData() > doesn't seem to check PGPROC entries for prepared transactions at all. > > In proc.c > -------------- > /* XXX allProcCount isn't really all of them; it excludes prepared xacts */ > ProcGlobal->allProcCount = MaxBackends + NUM_AUXILIARY_PROCS; > --------------
Oh right, thanks for pointing out! > I removed the period at the end to match the usual convention in the codebase > for single-line comment. > > I've attached v2 patch. Thanks for the new version! > You mean adding an assertion check to ensure that the slot ID calculated by > FAST_PATH_SLOT() is less than FP_LOCK_SLOTS_PER_BACKEND? Yes. > But GetLockStatusData() > already calls FAST_PATH_GET_BITS() right after FAST_PATH_SLOT(), > and FAST_PATH_GET_BITS() has an assertion that validates this. Oh right, it's "already" in FAST_PATH_GROUP() (and FAST_PATH_INDEX()). > So, probably > we can consider that this check is already in place. If it’s still worth > adding, > perhaps placing it inside the FAST_PATH_SLOT() macro could be an option... > Or current assertion check is enough? Thought? Given that it's already done in FAST_PATH_GET_BITS(), I think that's fine as it is and v2 LGTM. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
