public inbox for [email protected]
help / color / mirror / Atom feedFrom: Richard Guo <[email protected]>
To: Soumyadeep Chakraborty <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Ashwin Agrawal <[email protected]>
Subject: Re: brininsert optimization opportunity
Date: Mon, 27 Nov 2023 15:37:20 +0800
Message-ID: <CAMbWs4_LLOoWWtnL1Va1qaHHiuvhH2kZY3EDWA7QqycEdRbFzw@mail.gmail.com> (raw)
In-Reply-To: <CAE-ML+8Nx1X7PL-HHeGq=BbzsXotpzZQKe7QQOtTmiGRS6CmQg@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CAE-ML+_ecaX4jRQZaRdzdTk4zGPY5qUDB6Y3eV=KQu--k49yeQ@mail.gmail.com>
<[email protected]>
<CAE-ML+-9K2h2R7zcvQGq8FarxvSVhgAGZkgH57iQCnWQoxOFww@mail.gmail.com>
<[email protected]>
<CAE-ML+9JVD3vt7JZtmNy8hOFQKtP2jxJtf27=kE=ViGyv36V3w@mail.gmail.com>
<CAE-ML+_jerNg64mSD5VYy=bOxcRQSr-QfESHvCVFPEwMM_jufQ@mail.gmail.com>
<CAE-ML+-nBfDXQEfh0Ex0bp=tZdzzs7y5bvmQJg1WC7KpJSZVmQ@mail.gmail.com>
<CAE-ML+_9Adp8rRfZ9FP4ODtQZLZcMaBvae5ACO9H+-FZdk_v9w@mail.gmail.com>
<[email protected]>
<CAEze2WhR0zkCTYrs1_-xJErcZkyKLnV-9=cKq43D6EURj6OSgQ@mail.gmail.com>
<CAE-ML+_1nGVH7CitWieLZNR3W54V+RUGG8RQEon5EdEHucGEFw@mail.gmail.com>
<[email protected]>
<CAMbWs4-w9qC-o9hQox9UHvdVZAYTp8OrPQOKtwbvzWaRejTT=Q@mail.gmail.com>
<CAE-ML+8Nx1X7PL-HHeGq=BbzsXotpzZQKe7QQOtTmiGRS6CmQg@mail.gmail.com>
On Mon, Nov 27, 2023 at 1:53 PM Soumyadeep Chakraborty <
[email protected]> wrote:
> On Sun, Nov 26, 2023 at 9:28 PM Richard Guo <[email protected]>
> wrote:
> > It seems that we have an oversight in this commit. If there is no tuple
> > that has been inserted, we wouldn't have an available insert state in
> > the clean up phase. So the Assert in brininsertcleanup() is not always
> > right. For example:
> >
> > regression=# update brin_summarize set value = brin_summarize.value;
> > server closed the connection unexpectedly
>
> I wasn't able to repro the issue on
> 86b64bafc19c4c60136a4038d2a8d1e6eecc59f2.
> with UPDATE/INSERT:
>
> This could be because since c5b7ba4e67aeb5d6f824b74f94114d99ed6e42b7,
> we have moved ExecOpenIndices()
> from ExecInitModifyTable() to ExecInsert(). Since we never open the
> indices if nothing is
> inserted, we would never attempt to close them with ExecCloseIndices()
> while the ii_AmCache
> is NULL (which is what causes this assertion failure).
AFAICS we would also open the indices from ExecUpdate(). So if we
update the table in a way that no new tuples are inserted, we will have
this issue. As I showed previously, the query below crashes for me on
latest master (dc9f8a7983).
regression=# update brin_summarize set value = brin_summarize.value;
server closed the connection unexpectedly
There are other code paths that call ExecOpenIndices(), such as
ExecMerge(). I believe it's not hard to create queries that trigger this
Assert for those cases.
Thanks
Richard
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], [email protected], [email protected]
Subject: Re: brininsert optimization opportunity
In-Reply-To: <CAMbWs4_LLOoWWtnL1Va1qaHHiuvhH2kZY3EDWA7QqycEdRbFzw@mail.gmail.com>
* 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