Re: Diagnostic comment in LogicalIncreaseXminForSlot

On Fri, May 21, 2021 at 6:00 PM Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>
>
>
> On Fri, May 21, 2021 at 11:26 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>
>> On Thu, May 20, 2021 at 5:43 PM Ashutosh Bapat
>> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>> >
>> > Hi
>> > LogicalIncreaseRestartDecodingForSlot() has a debug log to report a
>> > new restart_lsn. But the corresponding function for catalog_xmin.
>> > Here's a patch to add the same.
>> >
>>
>> I think this can be useful. One minor comment:
>> + elog(DEBUG1, "got new catalog_xmin %u at %X/%X", xmin,
>> + (uint32) (current_lsn >> 32), (uint32) current_lsn);
>>
>> Isn't it better to use LSN_FORMAT_ARGS for current_lsn?
>
>
> Thanks for reminding me about that. :).
>
> Attached revised patch.
>
>>
>> Also, there
>> doesn't seem to be any urgency for adding this, so you can register it
>> for the next CF so that we can add this when the branch opens for
>> PG-15.
>
>
> It's there in CF. I am fine with PG-15. It will be good to patch the back-branches to have this extra diagnostic information available.

The patch looks to me.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/