Skip to content

Commit b6bea36

Browse files
author
Artem Fadeev
committed
Fix smart statement timeout update logic and aqo_stat_store
Note: due to a mix of absolute and relative time in set_timeout_if_need function, smart statement timeout feature doesn't currently work since its timeouts are set in the past. This commit changes checked precondition for smart statement timeout change to fix array indexing bug, but the feature itself remains broken. This commit also fixes arithmetic errors in aqo_stat_store in the case of fully filled arrays.
1 parent 294f523 commit b6bea36

File tree

5 files changed

+254
-15
lines changed

5 files changed

+254
-15
lines changed

Diff for: ‎expected/aqo_query_stat.out

+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
-- Testing aqo_query_stat update logic
2+
-- Note: this test assumes STAT_SAMPLE_SIZE to be 20.
3+
CREATE EXTENSION IF NOT EXISTS aqo;
4+
SELECT true AS success FROM aqo_reset();
5+
success
6+
---------
7+
t
8+
(1 row)
9+
10+
DROP TABLE IF EXISTS A;
11+
NOTICE: table "a" does not exist, skipping
12+
CREATE TABLE A AS SELECT x FROM generate_series(1, 20) as x;
13+
ANALYZE A;
14+
DROP TABLE IF EXISTS B;
15+
NOTICE: table "b" does not exist, skipping
16+
CREATE TABLE B AS SELECT y FROM generate_series(1, 10) as y;
17+
ANALYZE B;
18+
CREATE OR REPLACE FUNCTION round_array (double precision[])
19+
RETURNS double precision[]
20+
LANGUAGE SQL
21+
AS $$
22+
SELECT array_agg(round(elem::numeric, 3))
23+
FROM unnest($1) as arr(elem);
24+
$$
25+
SET aqo.mode = 'learn';
26+
SET aqo.force_collect_stat = 'on';
27+
SET aqo.min_neighbors_for_predicting = 1;
28+
-- First test: adding real records
29+
SET aqo.mode = 'disabled';
30+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 15 AND B.y < 5;
31+
count
32+
-------
33+
20
34+
(1 row)
35+
36+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 16 AND B.y < 6;
37+
count
38+
-------
39+
20
40+
(1 row)
41+
42+
SET aqo.mode = 'learn';
43+
SELECT aqo_enable_class(queryid) FROM aqo_queries WHERE queryid != 0;
44+
aqo_enable_class
45+
------------------
46+
47+
(1 row)
48+
49+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 17 AND B.y < 7;
50+
count
51+
-------
52+
18
53+
(1 row)
54+
55+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 18 AND B.y < 8;
56+
count
57+
-------
58+
14
59+
(1 row)
60+
61+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 19 AND B.y < 9;
62+
count
63+
-------
64+
8
65+
(1 row)
66+
67+
-- Ignore unstable time-related columns
68+
SELECT round_array(cardinality_error_with_aqo) AS error_aqo, round_array(cardinality_error_without_aqo) AS error_no_aqo, executions_with_aqo, executions_without_aqo FROM aqo_query_stat;
69+
error_aqo | error_no_aqo | executions_with_aqo | executions_without_aqo
70+
--------------------+--------------+---------------------+------------------------
71+
{0.22,0.362,0.398} | {0.392,0.21} | 3 | 2
72+
(1 row)
73+
74+
SELECT true AS success from aqo_reset();
75+
success
76+
---------
77+
t
78+
(1 row)
79+
80+
-- Second test: fake data in aqo_query_stat
81+
SET aqo.mode = 'disabled';
82+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 5 AND B.y < 100;
83+
count
84+
-------
85+
135
86+
(1 row)
87+
88+
SELECT aqo_query_stat_update(
89+
queryid,
90+
'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}', '{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}',
91+
'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}', '{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}',
92+
'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}', '{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}',
93+
100, 50)
94+
FROM aqo_query_stat;
95+
aqo_query_stat_update
96+
-----------------------
97+
t
98+
(1 row)
99+
100+
SELECT round_array(cardinality_error_with_aqo) AS error_aqo, round_array(cardinality_error_without_aqo) AS error_no_aqo, executions_with_aqo, executions_without_aqo FROM aqo_query_stat;
101+
error_aqo | error_no_aqo | executions_with_aqo | executions_without_aqo
102+
------------------------------------------------------+------------------------------------------------------+---------------------+------------------------
103+
{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20} | {1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20} | 100 | 50
104+
(1 row)
105+
106+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 10 AND B.y < 100;
107+
count
108+
-------
109+
100
110+
(1 row)
111+
112+
SET aqo.mode = 'learn';
113+
SELECT aqo_enable_class(queryid) FROM aqo_queries WHERE queryid != 0;
114+
aqo_enable_class
115+
------------------
116+
117+
(1 row)
118+
119+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 15 AND B.y < 5;
120+
count
121+
-------
122+
20
123+
(1 row)
124+
125+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 16 AND B.y < 6;
126+
count
127+
-------
128+
20
129+
(1 row)
130+
131+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 17 AND B.y < 7;
132+
count
133+
-------
134+
18
135+
(1 row)
136+
137+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 18 AND B.y < 8;
138+
count
139+
-------
140+
14
141+
(1 row)
142+
143+
SELECT round_array(cardinality_error_with_aqo) AS error_aqo, round_array(cardinality_error_without_aqo) AS error_no_aqo, executions_with_aqo, executions_without_aqo FROM aqo_query_stat;
144+
error_aqo | error_no_aqo | executions_with_aqo | executions_without_aqo
145+
---------------------------------------------------------------------+----------------------------------------------------------+---------------------+------------------------
146+
{5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,0.392,0.344,0.34,0.362} | {2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,0.218} | 104 | 51
147+
(1 row)
148+
149+
SET aqo.mode TO DEFAULT;
150+
SET aqo.force_collect_stat TO DEFAULT;
151+
SET aqo.min_neighbors_for_predicting TO DEFAULT;
152+
DROP FUNCTION round_array;
153+
DROP TABLE A;
154+
DROP TABLE B;
155+
DROP EXTENSION aqo CASCADE;

Diff for: ‎postprocessing.c

+13-8
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
#include "machine_learning.h"
3131
#include "storage.h"
3232

33+
#define SMART_TIMEOUT_ERROR_THRESHOLD (0.1)
34+
3335

3436
bool aqo_learn_statement_timeout = false;
3537

@@ -761,7 +763,6 @@ aqo_ExecutorEnd(QueryDesc *queryDesc)
761763
instr_time endtime;
762764
EphemeralNamedRelation enr = get_ENR(queryDesc->queryEnv, PlanStateInfo);
763765
MemoryContext oldctx = MemoryContextSwitchTo(AQOLearnMemCtx);
764-
double error = .0;
765766

766767
cardinality_sum_errors = 0.;
767768
cardinality_num_objects = 0;
@@ -827,18 +828,22 @@ aqo_ExecutorEnd(QueryDesc *queryDesc)
827828

828829
if (stat != NULL)
829830
{
830-
/* Store all learn data into the AQO service relations. */
831-
if (!query_context.adding_query && query_context.auto_tuning)
832-
automatical_query_tuning(query_context.query_hash, stat);
833-
834-
error = stat->est_error_aqo[stat->cur_stat_slot_aqo-1] - cardinality_sum_errors/(1 + cardinality_num_objects);
835-
836-
if ( aqo_learn_statement_timeout_enable && aqo_statement_timeout > 0 && error >= 0.1)
831+
Assert(!query_context.use_aqo || stat->cur_stat_slot_aqo > 0);
832+
/* If query used aqo, increase smart timeout if needed */
833+
if (query_context.use_aqo &&
834+
aqo_learn_statement_timeout_enable &&
835+
aqo_statement_timeout > 0 &&
836+
stat->est_error_aqo[stat->cur_stat_slot_aqo-1] -
837+
cardinality_sum_errors/(1 + cardinality_num_objects) >= SMART_TIMEOUT_ERROR_THRESHOLD)
837838
{
838839
int64 fintime = increase_smart_timeout();
839840
elog(NOTICE, "[AQO] Time limit for execution of the statement was increased. Current timeout is "UINT64_FORMAT, fintime);
840841
}
841842

843+
/* Store all learn data into the AQO service relations. */
844+
if (!query_context.adding_query && query_context.auto_tuning)
845+
automatical_query_tuning(query_context.query_hash, stat);
846+
842847
pfree(stat);
843848
}
844849
}

Diff for: ‎regress_schedule

+1
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,4 @@ test: look_a_like
2323
test: feature_subspace
2424
test: eclasses
2525
test: eclasses_mchar
26+
test: aqo_query_stat

Diff for: ‎sql/aqo_query_stat.sql

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
-- Testing aqo_query_stat update logic
2+
-- Note: this test assumes STAT_SAMPLE_SIZE to be 20.
3+
CREATE EXTENSION IF NOT EXISTS aqo;
4+
SELECT true AS success FROM aqo_reset();
5+
6+
DROP TABLE IF EXISTS A;
7+
CREATE TABLE A AS SELECT x FROM generate_series(1, 20) as x;
8+
ANALYZE A;
9+
10+
DROP TABLE IF EXISTS B;
11+
CREATE TABLE B AS SELECT y FROM generate_series(1, 10) as y;
12+
ANALYZE B;
13+
14+
CREATE OR REPLACE FUNCTION round_array (double precision[])
15+
RETURNS double precision[]
16+
LANGUAGE SQL
17+
AS $$
18+
SELECT array_agg(round(elem::numeric, 3))
19+
FROM unnest($1) as arr(elem);
20+
$$
21+
22+
SET aqo.mode = 'learn';
23+
SET aqo.force_collect_stat = 'on';
24+
SET aqo.min_neighbors_for_predicting = 1;
25+
26+
-- First test: adding real records
27+
SET aqo.mode = 'disabled';
28+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 15 AND B.y < 5;
29+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 16 AND B.y < 6;
30+
31+
SET aqo.mode = 'learn';
32+
SELECT aqo_enable_class(queryid) FROM aqo_queries WHERE queryid != 0;
33+
34+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 17 AND B.y < 7;
35+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 18 AND B.y < 8;
36+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 19 AND B.y < 9;
37+
-- Ignore unstable time-related columns
38+
SELECT round_array(cardinality_error_with_aqo) AS error_aqo, round_array(cardinality_error_without_aqo) AS error_no_aqo, executions_with_aqo, executions_without_aqo FROM aqo_query_stat;
39+
40+
SELECT true AS success from aqo_reset();
41+
42+
43+
-- Second test: fake data in aqo_query_stat
44+
SET aqo.mode = 'disabled';
45+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 5 AND B.y < 100;
46+
SELECT aqo_query_stat_update(
47+
queryid,
48+
'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}', '{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}',
49+
'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}', '{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}',
50+
'{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}', '{1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20}',
51+
100, 50)
52+
FROM aqo_query_stat;
53+
SELECT round_array(cardinality_error_with_aqo) AS error_aqo, round_array(cardinality_error_without_aqo) AS error_no_aqo, executions_with_aqo, executions_without_aqo FROM aqo_query_stat;
54+
55+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 10 AND B.y < 100;
56+
57+
SET aqo.mode = 'learn';
58+
SELECT aqo_enable_class(queryid) FROM aqo_queries WHERE queryid != 0;
59+
60+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 15 AND B.y < 5;
61+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 16 AND B.y < 6;
62+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 17 AND B.y < 7;
63+
SELECT count(*) FROM A JOIN B ON (A.x > B.y) WHERE A.x > 18 AND B.y < 8;
64+
SELECT round_array(cardinality_error_with_aqo) AS error_aqo, round_array(cardinality_error_without_aqo) AS error_no_aqo, executions_with_aqo, executions_without_aqo FROM aqo_query_stat;
65+
66+
67+
SET aqo.mode TO DEFAULT;
68+
SET aqo.force_collect_stat TO DEFAULT;
69+
SET aqo.min_neighbors_for_predicting TO DEFAULT;
70+
71+
DROP FUNCTION round_array;
72+
DROP TABLE A;
73+
DROP TABLE B;
74+
DROP EXTENSION aqo CASCADE;

Diff for: ‎storage.c

+11-7
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,9 @@ reset_deactivated_queries(void)
233233
/*
234234
* Update AQO statistics.
235235
*
236-
* Add a record (or update an existed) to stat storage for the query class.
236+
* In append mode, append one element to exec_time, plan_time, est_error arrays
237+
* (or their *_aqo counterparts, if use_aqo is true). Without append mode, add a
238+
* record (or overwrite an existing) to stat storage for the query class.
237239
* Returns a copy of stat entry, allocated in current memory context. Caller is
238240
* in charge to free this struct after usage.
239241
* If stat hash table is full, return NULL and log this fact.
@@ -312,19 +314,20 @@ aqo_stat_store(uint64 queryid, bool use_aqo, AqoStatArgs *stat_arg,
312314
if (use_aqo)
313315
{
314316
Assert(entry->cur_stat_slot_aqo >= 0);
315-
pos = entry->cur_stat_slot_aqo;
316-
if (entry->cur_stat_slot_aqo < STAT_SAMPLE_SIZE - 1)
317+
if (entry->cur_stat_slot_aqo < STAT_SAMPLE_SIZE)
317318
entry->cur_stat_slot_aqo++;
318319
else
319320
{
320321
size_t sz = (STAT_SAMPLE_SIZE - 1) * sizeof(entry->est_error_aqo[0]);
321322

322-
Assert(entry->cur_stat_slot_aqo = STAT_SAMPLE_SIZE - 1);
323+
Assert(entry->cur_stat_slot_aqo == STAT_SAMPLE_SIZE);
324+
323325
memmove(entry->plan_time_aqo, &entry->plan_time_aqo[1], sz);
324326
memmove(entry->exec_time_aqo, &entry->exec_time_aqo[1], sz);
325327
memmove(entry->est_error_aqo, &entry->est_error_aqo[1], sz);
326328
}
327329

330+
pos = entry->cur_stat_slot_aqo - 1;
328331
entry->execs_with_aqo++;
329332
entry->plan_time_aqo[pos] = *stat_arg->plan_time_aqo;
330333
entry->exec_time_aqo[pos] = *stat_arg->exec_time_aqo;
@@ -333,19 +336,20 @@ aqo_stat_store(uint64 queryid, bool use_aqo, AqoStatArgs *stat_arg,
333336
else
334337
{
335338
Assert(entry->cur_stat_slot >= 0);
336-
pos = entry->cur_stat_slot;
337-
if (entry->cur_stat_slot < STAT_SAMPLE_SIZE - 1)
339+
if (entry->cur_stat_slot < STAT_SAMPLE_SIZE)
338340
entry->cur_stat_slot++;
339341
else
340342
{
341343
size_t sz = (STAT_SAMPLE_SIZE - 1) * sizeof(entry->est_error[0]);
342344

343-
Assert(entry->cur_stat_slot = STAT_SAMPLE_SIZE - 1);
345+
Assert(entry->cur_stat_slot == STAT_SAMPLE_SIZE);
346+
344347
memmove(entry->plan_time, &entry->plan_time[1], sz);
345348
memmove(entry->exec_time, &entry->exec_time[1], sz);
346349
memmove(entry->est_error, &entry->est_error[1], sz);
347350
}
348351

352+
pos = entry->cur_stat_slot - 1;
349353
entry->execs_without_aqo++;
350354
entry->plan_time[pos] = *stat_arg->plan_time;
351355
entry->exec_time[pos] = *stat_arg->exec_time;

0 commit comments

Comments
 (0)