Re: pgsql: Add function to log the memory contexts of specified backend pro

On 2025/05/07 18:06, Fujii Masao wrote:
>
>
> On 2025/05/05 23:57, Robert Haas wrote:
>> On Fri, May 2, 2025 at 9:54 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>> Thanks for the review and testing! I've fixed the issue you pointed out.
>>> Updated patch attached.
>>
>> Thanks for addressing this. However, I believe this commit may need to
>> take note of the following comment from elog.h:
>
> Thanks for the review!
>
>
>>   * Note: if a local variable of the function containing PG_TRY is modified
>>   * in the PG_TRY section and used in the PG_CATCH section, that variable
>>   * must be declared "volatile" for POSIX compliance.  This is not mere
>>   * pedantry; we have seen bugs from compilers improperly optimizing code
>>   * away when such a variable was not marked.  Beware that gcc's -Wclobbered
>>   * warnings are just about entirely useless for catching such oversights.
>>
>> Based on this comment, I believe in_progress must be declared volatile.
>
> You're right. OTOH, setting the flag inside the PG_TRY() block isn't necessary,
> so I've moved it outside instead of leaving it inside and marking the flag volatile.
>
>
>> As a stylistic comment, I think I would prefer making in_progress a
>> file-level global and giving it a less generic name (e.g.
>> LogMemoryContextInProgress). However, perhaps others will disagree.
>
> I'm fine with this. I've renamed the flag and made it a file-level global
> variable as suggested. Updated patch is attached.

I've attached the rebased versions of the patches.

The patch for v14–v16 is labeled with a .txt extension to prevent cfbot
from treating it as a patch for master, which would cause it to fail
when applying.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation