Date: Fri, 17 Jun 2022 17:25:49 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	pgsql-hackers@postgresql.org
Subject: Re: should check interrupts in BuildRelationExtStatistics ?
Message-ID: <20220617222549.GI29853@telsasoft.com>
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>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="nqkreNcslJAfgyzk"
Content-Disposition: inline
In-Reply-To: <Yp2rdhPtSHDJxPAd@paquier.xyz>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220617222549.GI29853%40telsasoft.com>
Precedence: bulk


--nqkreNcslJAfgyzk
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--nqkreNcslJAfgyzk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-wip-check-for-interrupts-during-extended-stats.patch"

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.c
index 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


--nqkreNcslJAfgyzk--