Re: Diagnostic comment in LogicalIncreaseXminForSlot
От | Ashutosh Bapat |
---|---|
Тема | Re: Diagnostic comment in LogicalIncreaseXminForSlot |
Дата | |
Msg-id | CAGEoWWS6LvgDHuh_uKkGT9KKfv4c-OS7oyH-wHufTYb4M8xa4Q@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Diagnostic comment in LogicalIncreaseXminForSlot (Amit Kapila <amit.kapila16@gmail.com>) |
Ответы |
Re: Diagnostic comment in LogicalIncreaseXminForSlot
Re: Diagnostic comment in LogicalIncreaseXminForSlot Re: Diagnostic comment in LogicalIncreaseXminForSlot |
Список | pgsql-hackers |
On Mon, Jul 12, 2021 at 8:39 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Jul 5, 2021 at 12:54 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, May 21, 2021 at 6:00 PM Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
> >
> > 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.
>
{
slot->candidate_catalog_xmin = xmin;
slot->candidate_xmin_lsn = current_lsn;
+ elog(DEBUG1, "got new catalog_xmin %u at %X/%X", xmin,
+ LSN_FORMAT_ARGS(current_lsn));
}
SpinLockRelease(&slot->mutex);
Today, again looking at this patch, I don't think doing elog inside
spinlock is a good idea. We can either release spinlock before it or
use some variable to remember that we need to write such an elog and
do it after releasing the lock. What do you think?
The elog will be effective only under DEBUG1, otherwise it will be almost a NOOP. I am wondering whether it's worth adding a bool assignment and move the elog out only for DEBUG1. Anyway, will defer it to you.
I have noticed that
a nearby function LogicalIncreaseRestartDecodingForSlot() logs similar
information after releasing spinlock, so it is better to follow the
same here as well.
Now that you mention it, the code their looks rather suspicious :)
We acquire the spinlock at the beginning of the function but do not release it if (restart_lsn <= slot->data.restart_lsn) or if (current_lsn <= slot->data.confirmed_flush). I might be missing something there. But it looks unrelated.
--
Best Wishes,
Ashutosh
В списке pgsql-hackers по дате отправления: