Subject: Re: AssertLog instead of Assert in some places

From: Andres Freund <[email-masked]>
Date: Fri, 11 Aug 2023 10:57:23 -0700

Hi,

On 2023-08-11 17:59:37 +0530, Ashutosh Bapat wrote:
> Most of the Asserts are recoverable by rolling back the transaction
> without crashing the backend. So an elog(ERROR, ) is enough. But just
> by themselves elogs are compiled into non-debug binary and the
> condition check can waste CPU cycles esp. conditions are mostly true
> like the ones we use in Assert.
> 
> Attached patch combines Assert and elog(ERROR, ) so that when an
> Assert is triggered in assert-enabled binary, it will throw an error
> while keeping the backend intact. Thus it does not affect gdb session
> or psql session. These elogs do not make their way to non-assert
> binary so do not make it to production like Assert.

I am quite strongly against this. This will lead to assertions being hit in
tests without that being noticed, e.g. because they happen in a background
process that just restarts.

Greetings,

Andres Freund

---

From: Peter Geoghegan <[email-masked]>
Date: Fri, 11 Aug 2023 11:14:34 -0700

> process that just restarts.

Couldn't you say the same thing about defensive "can't happen" ERRORs?
They are essentially a form of assertion that isn't limited to
assert-enabled builds.

I have sometimes thought that it would be handy if there was a variant
of "can't happen" ERRORs that took on the structure of an assertion.
(This is quite different to what Ashutosh has proposed, though, since
it would still look like a conventional assertion failure on
assert-enabled builds.)

-- 
Peter Geoghegan

---

From: Ashutosh Bapat <[email-masked]>
Date: Mon, 14 Aug 2023 20:07:34 +0530

> process that just restarts.

Fair point. Our regression doesn't check server error logs for
unwanted errors. How about restricting it to only client backends? I
don't know how to identify those from others but there must be a way.

--
Best Wishes,
Ashutosh Bapat