Re: should check interrupts in BuildRelationExtStatistics ?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: should check interrupts in BuildRelationExtStatistics ?
Date: 2022-07-01 23:19:11
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> On Mon, Jun 06, 2022 at 04:23:34PM +0900, Michael Paquier wrote:
>> 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.

Hmm. On my machine, the example last shown upthread takes about 9
seconds, which I agree is a mighty long time to be unresponsive
--- but it appears that fully half of that elapses before we
reach multi_sort_compare for the first time. The first half of
the ANALYZE run does seem to contain some CFI calls, but they
are not exactly thick on the ground there either. So I'm feeling
like this patch isn't ambitious enough.

I tried interrupting at a random point and then stepping, and
look what I hit after just a couple of steps:

(gdb) s
qsort_arg (data=data(at)entry=0x13161410, n=<optimized out>, n(at)entry=1679616,
compare=compare(at)entry=0x649450 <compare_scalars>,
arg=arg(at)entry=0x7ffec539c0f0) at ../../src/include/lib/sort_template.h:353
353 if (r == 0)
358 pc -= ST_POINTER_STEP;

That, um, piqued my interest. After a bit of digging,
I modestly propose the attached. I'm not sure if it's
okay to back-patch this, because maybe someone out there
is relying on qsort() to be incapable of throwing an error
--- but it'd solve the problem at hand and a bunch of other
issues of the same ilk.

regards, tom lane

Attachment Content-Type Size
enable-check-for-interrupts-in-qsort.patch text/x-diff 1.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-07-01 23:20:09 Re: margay fails assertion in stats/dsa/dsm code
Previous Message Andres Freund 2022-07-01 23:18:33 Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?