Re: Diagnostic comment in LogicalIncreaseXminForSlot - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Diagnostic comment in LogicalIncreaseXminForSlot
Date
Msg-id CAGEoWWSbxeAjM0mNhpGoEcggAOqB1K4ODVQneM2GMsZ6aS12-A@mail.gmail.com
Whole thread Raw
In response to Re: Diagnostic comment in LogicalIncreaseXminForSlot  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Responses Re: Diagnostic comment in LogicalIncreaseXminForSlot  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
Hi Amit and Andres,
Here's updated patch

On Mon, Aug 9, 2021 at 11:14 AM Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:


On Sat, Aug 7, 2021 at 11:40 AM Andres Freund <andres@anarazel.de> wrote:
Hi,

On 2021-07-12 17:28:15 +0530, Ashutosh Bapat wrote:
> 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>
> > 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.

It's *definitely* not ok to do an elog() while holding a spinlock. Consider
what happens if the elog tries to format the message and runs out of
memory. Or if elog detects the stack depth is too deep. An ERROR would be
thrown, and we'd loose track of the held spinlock.

Thanks for the clarification.

Amit,
I will provide the patch changed accordingly.

--
Best Wishes,
Ashutosh


--
--
Best Wishes,
Ashutosh
Attachment

pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: A problem in ExecModifyTable
Next
From: Michael Paquier
Date:
Subject: Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE