Re: elog(DEBUG2 in SpinLocked section.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, masao(dot)fujii(at)oss(dot)nttdata(dot)com, amit(dot)kapila16(at)gmail(dot)com, pasim(at)vmware(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: elog(DEBUG2 in SpinLocked section.
Date: 2020-06-03 04:36:34
Message-ID: 939923.1591158994@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> Indeed, this was incorrect. And you may not have noticed, but we have
> a second instance of that in LogicalIncreaseRestartDecodingForSlot()
> that goes down to 9.4 and b89e151. I used a dirty-still-efficient
> hack to detect that, and that's the only instance I have spotted.

Ugh, that is just horrid. I experimented with the attached patch
but it did not find any other problems. Still, that only proves
something about code paths that are taken during check-world, and
we know that our test coverage is not very good :-(.

Should we think about adding automated detection of this type of
mistake? I don't like the attached as-is because of the #include
footprint expansion, but maybe we can find a better way.

> I am not sure if that's worth worrying a back-patch, but we should
> really address that at least on HEAD.

It's actually worse in the back branches, because elog() did not have
a good short-circuit path like ereport() does. +1 for back-patch.

regards, tom lane

Attachment Content-Type Size
detect-misuse-of-spinlocks.patch text/x-diff 3.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-06-03 04:41:21 Re: elog(DEBUG2 in SpinLocked section.
Previous Message Amit Kapila 2020-06-03 03:22:08 Re: elog(DEBUG2 in SpinLocked section.