compute_index_stats is missing a CHECK_FOR_INTERRUPTS
Analyze on functional indexes cannot be interrupted very easily.
Example:
create language plperl;
create table foo1 as select x::text from generate_series(1,1000) foo (x);
create table foo2 as select reverse(x) from foo1;
--use a fast version to set up the demo, as we are impatient
CREATE or replace FUNCTION slow_reverse(text) RETURNS text
LANGUAGE plperl IMMUTABLE STRICT COST 1000000
AS $_X$
return reverse($_[0]);
$_X$;
create index on foo2 (slow_reverse(reverse));
analyze foo2;
--put the slow version in place.
CREATE or replace FUNCTION slow_reverse(text) RETURNS text
LANGUAGE plperl IMMUTABLE STRICT COST 1000000
AS $_X$
my $foo; foreach (1..1e6) {$foo+=sqrt($_)};
return reverse($_[0]);
$_X$;
-- now spring the trap
analyze foo2;
Ctrl-C (or pg_ctl stop -mf) hangs for a long time.
The attached patch fixes it, but don't vouch for its safety.
I believe I've seen a real-world example of this causing refusal of a fast
shutdown to shutdown fast.
Cheers,
Jeff
Attachments:
compute_index_stats_interrupt.patchapplication/octet-stream; name=compute_index_stats_interrupt.patchDownload
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
new file mode 100644
index 366c4af..42b5fc8
*** a/src/backend/commands/analyze.c
--- b/src/backend/commands/analyze.c
*************** compute_index_stats(Relation onerel, dou
*** 748,753 ****
--- 748,755 ----
*/
ResetExprContext(econtext);
+ CHECK_FOR_INTERRUPTS();
+
/* Set up for predicate or expression evaluation */
ExecStoreTuple(heapTuple, slot, InvalidBuffer, false);
Jeff Janes <jeff.janes@gmail.com> writes:
Analyze on functional indexes cannot be interrupted very easily.
...
The attached patch fixes it, but don't vouch for its safety.
Hm. The other per-sample-row loops in analyze.c use vacuum_delay_point()
rather than CHECK_FOR_INTERRUPTS() directly. Ordinarily that wouldn't
make much difference here, but maybe a slow index function might be
incurring I/O?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Mar 28, 2015 at 3:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Jeff Janes <jeff.janes@gmail.com> writes:
Analyze on functional indexes cannot be interrupted very easily.
...
The attached patch fixes it, but don't vouch for its safety.Hm. The other per-sample-row loops in analyze.c use vacuum_delay_point()
rather than CHECK_FOR_INTERRUPTS() directly. Ordinarily that wouldn't
make much difference here, but maybe a slow index function might be
incurring I/O?
That isn't the case for me (and if it were, they wouldn't be going through
the buffer manager anyway and so would not trigger delay criteria), but
that seems like a valid concern in general. It also explains why I
couldn't find CHECK_FOR_INTERRUPTS in other loops of that file, because I
was looking for the wrong spelling.
Adding a vacuum_delay_point does solve the immediately observed problem,
both the toy one and the more realistic one.
Thanks,
Jeff
Attachments:
compute_index_stats_vacdelay.patchapplication/octet-stream; name=compute_index_stats_vacdelay.patchDownload
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
new file mode 100644
index 366c4af..d4d1914
*** a/src/backend/commands/analyze.c
--- b/src/backend/commands/analyze.c
*************** compute_index_stats(Relation onerel, dou
*** 742,747 ****
--- 742,749 ----
{
HeapTuple heapTuple = rows[rowno];
+ vacuum_delay_point();
+
/*
* Reset the per-tuple context each time, to reclaim any cruft
* left behind by evaluating the predicate or index expressions.
Jeff Janes <jeff.janes@gmail.com> writes:
On Sat, Mar 28, 2015 at 3:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Hm. The other per-sample-row loops in analyze.c use vacuum_delay_point()
rather than CHECK_FOR_INTERRUPTS() directly. Ordinarily that wouldn't
make much difference here, but maybe a slow index function might be
incurring I/O?
That isn't the case for me (and if it were, they wouldn't be going through
the buffer manager anyway and so would not trigger delay criteria), but
that seems like a valid concern in general. It also explains why I
couldn't find CHECK_FOR_INTERRUPTS in other loops of that file, because I
was looking for the wrong spelling.
Adding a vacuum_delay_point does solve the immediately observed problem,
both the toy one and the more realistic one.
Committed, thanks.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers