public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Hao Zhang <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] plpython function causes server panic
Date: Tue, 05 Dec 2023 16:56:33 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALY6Dr8fPasw5DP3FFD2YjRZMKeX9_Ank14tz_bmPjjqLB1U7g@mail.gmail.com>
References: <CALY6Dr-2yLVeVPhNMhuBnRgOZo1UjoTETgtKBx1B2gUi8yy+3g@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CALY6Dr8fPasw5DP3FFD2YjRZMKeX9_Ank14tz_bmPjjqLB1U7g@mail.gmail.com>

Hao Zhang <[email protected]> writes:
>> The only readily-reachable error case in BeginInternalSubTransaction
>> is this specific one about IsInParallelMode, which was added later
>> than the original design and evidently with not a lot of thought or
>> testing.  The comment for it speculates about whether we could get
>> rid of it, so I wonder if our thoughts about this ought to go in that
>> direction.

> IMHO, there are other error reports in the function
> BeginInternalSubTransaction(), like

Sure, but all the other ones are extremely hard to hit, which is why
we didn't bother to worry about them to begin with.  If we want to
make this more formally bulletproof, my inclination would be to
(a) get rid of the IsInParallelMode restriction and then (b) turn
the function into a critical section, so that any other error gets
treated as a PANIC.  Maybe at some point we'd be willing to make a
variant of BeginInternalSubTransaction that has a different API and
can manage such cases without a PANIC, but that seems far down the
road to me, and certainly not something to be back-patched.

The main reason for my caution here is that, by catching an error
and allowing Python (or Perl, or something else) code to decide
what to do next, we are very dependent on that code doing the right
thing.  This is already a bit of a leap of faith for run-of-the-mill
errors.  For errors in transaction startup or shutdown, I think it's
a bigger leap than I care to make.  We're pretty well hosed if we
can't make the transaction machinery work, so imagining that we can
clean up after such an error and march merrily onwards seems mighty
optimistic.

			regards, tom lane

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] plpython function causes server panic
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox