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
