Re: should check interrupts in BuildRelationExtStatistics ?

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,
element_size=element_size(at)entry=16,
compare=compare(at)entry=0x649450 <compare_scalars>,
arg=arg(at)entry=0x7ffec539c0f0) at ../../src/include/lib/sort_template.h:353
353 if (r == 0)
(gdb)
358 pc -= ST_POINTER_STEP;
(gdb)
359 DO_CHECK_FOR_INTERRUPTS();

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