From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Greg Stark <stark@mit.edu>
Cc: pgsql-hackers@postgresql.org
Subject: Re: identifying the backend that owns a temporary schema
Date: Thu, 29 Sep 2022 10:47:06 -0400
Message-ID: <68998.1664462826@sss.pgh.pa.us> (raw)
In-Reply-To: <20220929024038.GA4945@nathanxps13>
References: <20220815205811.GA250990@nathanxps13>
<CAM-w4HMZ03AbQjBbfN_mTxct68HX3NPRQVfLaRxLGJTejJ8xbg@mail.gmail.com>
<20220829230757.GA542616@nathanxps13>
<2297420.1664041298@sss.pgh.pa.us>
<20220926160822.GA1298186@nathanxps13>
<3499642.1664221809@sss.pgh.pa.us>
<20220926201113.GA1340606@nathanxps13>
<4157446.1664405780@sss.pgh.pa.us>
<20220929024038.GA4945@nathanxps13>
Nathan Bossart <nathandbossart@gmail.com> writes:
> On Wed, Sep 28, 2022 at 06:56:20PM -0400, Tom Lane wrote:
>> A point that still bothers me a bit about pg_stat_get_backend_idset is
>> that it could miss or duplicate some backend IDs if the user calls
>> pg_stat_clear_snapshot() partway through the SRF's run, and we reload
>> a different set of backend entries than we had before. I added a comment
>> about that, with an argument why it's not worth working harder, but
>> is the argument convincing? If not, what should we do?
> Isn't this an existing problem? Granted, it'd manifest differently with
> this patch, but ISTM we could already end up with too many or too few
> backend IDs if there's a refresh partway through.
Right. I'd been thinking the current code wouldn't generate duplicate IDs
--- but it might produce duplicate query output anyway, in case a given
backend's entry is later in the array than it was before. So really
there's not much guarantees here in any case.
>> - if (beid < 1 || beid > localNumBackends)
>> - return NULL;
> The reason I'd kept this in was because I was worried about overflow in the
> comparator function. Upon further inspection, I don't think there's
> actually any way that will produce incorrect results. And I'm not sure we
> should worry about such cases too much, anyway.
Ah, I see: if the user passes in a "backend ID" that is close to INT_MIN,
then the comparator's subtraction could overflow. We could fix that by
writing out the comparator code longhand ("if (a < b) return -1;" etc),
but I don't really think it's necessary. bsearch is guaranteed to
correctly report that such a key is not present, even if it takes a
strange search path through the array due to inconsistent comparator
results. So the test quoted above just serves to fail a bit more quickly,
but we certainly shouldn't be optimizing for the case of a bad ID.
> Overall, LGTM.
OK. Will push shortly.
regards, tom lane
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: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, nathandbossart@gmail.com, stark@mit.edu
Subject: Re: identifying the backend that owns a temporary schema
In-Reply-To: <68998.1664462826@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox