From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: should check interrupts in BuildRelationExtStatistics ?
Date: Fri, 17 Jun 2022 17:25:49 -0500
Message-ID: <20220617222549.GI29853@telsasoft.com> (raw)
In-Reply-To: <Yp2rdhPtSHDJxPAd@paquier.xyz>
References: <20220509000108.GQ28830@telsasoft.com>
<YniLBN/x8N97ZDzf@paquier.xyz>
<3592964.1652067393@sss.pgh.pa.us>
<CA+TgmoY=Q9C9n=P=e8gTqLFTLza89t-cz+X-16LPaxq2+=qY2g@mail.gmail.com>
<20220603152837.GO29853@telsasoft.com>
<20220605014233.GR29853@telsasoft.com>
<Yp2rdhPtSHDJxPAd@paquier.xyz>
On Mon, Jun 06, 2022 at 04:23:34PM +0900, Michael Paquier wrote:
> On Sat, Jun 04, 2022 at 08:42:33PM -0500, Justin Pryzby wrote:
> > The fix seems to be to CHECK_FOR_INTERRUPTS() within multi_sort_compare().
> > That would supercede the other two CHECK_FOR_INTERRUPTS I'd proposed, and
> > handle mcv, depends, and ndistinct all at once.
>
> Hmm. I have to admit that adding a CFI() in multi_sort_compare()
> stresses me a bit as it is dependent on the number of rows involved,
> and it can be used as a qsort routine.
That's exactly the problem for which I showed a backtrace - it took 10s of
seconds to do qsort, which is (uh) a human timescale and too long to be
unresponsive, even if I create on a table with many rows a stats object with a
lot of columns and a high stats target.
Attachments:
[text/x-diff] 0001-wip-check-for-interrupts-during-extended-stats.patch (948B, ../20220617222549.GI29853@telsasoft.com/2-0001-wip-check-for-interrupts-during-extended-stats.patch)
download | inline diff:
From 1c5fe1fb1cf4d2e38cf3b8edef1288cb63388cc4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 8 May 2022 19:36:43 -0500
Subject: [PATCH] wip: check for interrupts during extended stats
It's possible to handle this partially by adding CFI at higher levels
(statext_dependencies_build and statext_ndistinct_build), but in order to
handle MCV, CFI has to be done at a low level here.
---
src/backend/statistics/extended_stats.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.cindex cb0a22b73e8..c6ba352e414 100644--- a/src/backend/statistics/extended_stats.c+++ b/src/backend/statistics/extended_stats.c@@ -894,6 +894,8 @@ multi_sort_compare(const void *a, const void *b, void *arg)
SortItem *ib = (SortItem *) b;
int i;
+ CHECK_FOR_INTERRUPTS();+
for (i = 0; i < mss->ndims; i++)
{
int compare;
--
2.17.1
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: pryzby@telsasoft.com, michael@paquier.xyz, robertmhaas@gmail.com, tgl@sss.pgh.pa.us, tomas.vondra@enterprisedb.com
Subject: Re: should check interrupts in BuildRelationExtStatistics ?
In-Reply-To: <20220617222549.GI29853@telsasoft.com>
* 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