Skip to content

Commit 6fd5071

Browse files
michaelpqmichaelpq
committed
Set query ID in parallel workers for vacuum, BRIN and btree
All these code paths use their own entry point when starting parallel workers, but failed to set a query ID, even if they set a text query. Hence, this data would be missed in pg_stat_activity for the worker processes. The main entry point for parallel query processing, ParallelQueryMain(), is already doing that by saving its query ID in a dummy PlannedStmt, but not the others. The code is changed so as the query ID of these queries is set in their shared state, and reported back once the parallel workers start. Some tests are added to show how the failures can happen for btree and BRIN with a parallel build enforced, which are able to trigger a failure in an assertion added by 24f5205 in the recovery TAP test 027_stream_regress.pl where pg_stat_statements is always loaded. In this case, the executor path was taken because the index expression needs to be flattened when building its IndexInfo. Alexander Lakhin has noticed the problem in btree, and I have noticed that the issue was more spread. This is arguably a bug, but nobody has complained about that until now, so no backpatch is done out of caution. If folks would like to see a backpatch, well, let me know. Reported-by: Alexander Lakhin Reviewed-by: Sami Imseih Discussion: https://postgr.es/m/cf3547c1-498a-6a61-7b01-819f902a251f@gmail.com
1 parent 0d5a3d7 commit 6fd5071

File tree

7 files changed

+64
-3
lines changed

7 files changed

+64
-3
lines changed

‎src/backend/access/brin/brin.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ typedef struct BrinShared
6767
BlockNumber pagesPerRange;
6868
int scantuplesortstates;
6969

70+
/* Query ID, for report in worker processes */
71+
uint64 queryid;
72+
7073
/*
7174
* workersdonecv is used to monitor the progress of workers. All parallel
7275
* participants must indicate that they are done before leader can use
@@ -2448,6 +2451,7 @@ _brin_begin_parallel(BrinBuildState *buildstate, Relation heap, Relation index,
24482451
brinshared->isconcurrent = isconcurrent;
24492452
brinshared->scantuplesortstates = scantuplesortstates;
24502453
brinshared->pagesPerRange = buildstate->bs_pagesPerRange;
2454+
brinshared->queryid = pgstat_get_my_query_id();
24512455
ConditionVariableInit(&brinshared->workersdonecv);
24522456
SpinLockInit(&brinshared->mutex);
24532457

@@ -2891,6 +2895,9 @@ _brin_parallel_build_main(dsm_segment *seg, shm_toc *toc)
28912895
indexLockmode = RowExclusiveLock;
28922896
}
28932897

2898+
/* Track query ID */
2899+
pgstat_report_query_id(brinshared->queryid, false);
2900+
28942901
/* Open relations within worker */
28952902
heapRel = table_open(brinshared->heaprelid, heapLockmode);
28962903
indexRel = index_open(brinshared->indexrelid, indexLockmode);

‎src/backend/access/nbtree/nbtsort.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ typedef struct BTShared
105105
bool isconcurrent;
106106
int scantuplesortstates;
107107

108+
/* Query ID, for report in worker processes */
109+
uint64 queryid;
110+
108111
/*
109112
* workersdonecv is used to monitor the progress of workers. All parallel
110113
* participants must indicate that they are done before leader can use
@@ -1505,6 +1508,7 @@ _bt_begin_parallel(BTBuildState *buildstate, bool isconcurrent, int request)
15051508
btshared->nulls_not_distinct = btspool->nulls_not_distinct;
15061509
btshared->isconcurrent = isconcurrent;
15071510
btshared->scantuplesortstates = scantuplesortstates;
1511+
btshared->queryid = pgstat_get_my_query_id();
15081512
ConditionVariableInit(&btshared->workersdonecv);
15091513
SpinLockInit(&btshared->mutex);
15101514
/* Initialize mutable state */
@@ -1787,6 +1791,9 @@ _bt_parallel_build_main(dsm_segment *seg, shm_toc *toc)
17871791
indexLockmode = RowExclusiveLock;
17881792
}
17891793

1794+
/* Track query ID */
1795+
pgstat_report_query_id(btshared->queryid, false);
1796+
17901797
/* Open relations within worker */
17911798
heapRel = table_open(btshared->heaprelid, heapLockmode);
17921799
indexRel = index_open(btshared->indexrelid, indexLockmode);

‎src/backend/commands/vacuumparallel.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,13 @@
5757
typedef struct PVShared
5858
{
5959
/*
60-
* Target table relid and log level (for messages about parallel workers
61-
* launched during VACUUM VERBOSE). These fields are not modified during
62-
* the parallel vacuum.
60+
* Target table relid, log level (for messages about parallel workers
61+
* launched during VACUUM VERBOSE) and query ID. These fields are not
62+
* modified during the parallel vacuum.
6363
*/
6464
Oid relid;
6565
int elevel;
66+
uint64 queryid;
6667

6768
/*
6869
* Fields for both index vacuum and cleanup.
@@ -369,6 +370,7 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
369370
MemSet(shared, 0, est_shared_len);
370371
shared->relid = RelationGetRelid(rel);
371372
shared->elevel = elevel;
373+
shared->queryid = pgstat_get_my_query_id();
372374
shared->maintenance_work_mem_worker =
373375
(nindexes_mwm > 0) ?
374376
maintenance_work_mem / Min(parallel_workers, nindexes_mwm) :
@@ -1014,6 +1016,9 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
10141016
debug_query_string = sharedquery;
10151017
pgstat_report_activity(STATE_RUNNING, debug_query_string);
10161018

1019+
/* Track query ID */
1020+
pgstat_report_query_id(shared->queryid, false);
1021+
10171022
/*
10181023
* Open table. The lock mode is the same as the leader process. It's
10191024
* okay because the lock mode does not conflict among the parallel

‎src/test/regress/expected/brin.out

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,16 @@ SELECT * FROM brintest_3 WHERE b < '0';
567567

568568
DROP TABLE brintest_3;
569569
RESET enable_seqscan;
570+
-- test parallel build with immutable function.
571+
CREATE TABLE brintest_expr (n int);
572+
CREATE FUNCTION brintest_func() RETURNS int LANGUAGE sql IMMUTABLE RETURN 0;
573+
BEGIN;
574+
SET LOCAL min_parallel_table_scan_size = 0;
575+
SET LOCAL max_parallel_maintenance_workers = 4;
576+
CREATE INDEX brintest_expr_idx ON brintest_expr USING brin (brintest_func());
577+
COMMIT;
578+
DROP TABLE brintest_expr;
579+
DROP FUNCTION brintest_func();
570580
-- test an unlogged table, mostly to get coverage of brinbuildempty
571581
CREATE UNLOGGED TABLE brintest_unlogged (n numrange);
572582
CREATE INDEX brinidx_unlogged ON brintest_unlogged USING brin (n);

‎src/test/regress/expected/btree_index.out

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,16 @@ INSERT INTO delete_test_table SELECT i, 1, 2, 3 FROM generate_series(1,1000) i;
476476
-- Test unsupported btree opclass parameters
477477
create index on btree_tall_tbl (id int4_ops(foo=1));
478478
ERROR: operator class int4_ops has no options
479+
-- test parallel build with immutable function.
480+
CREATE TABLE btree_test_expr (n int);
481+
CREATE FUNCTION btree_test_func() RETURNS int LANGUAGE sql IMMUTABLE RETURN 0;
482+
BEGIN;
483+
SET LOCAL min_parallel_table_scan_size = 0;
484+
SET LOCAL max_parallel_maintenance_workers = 4;
485+
CREATE INDEX btree_test_expr_idx ON btree_test_expr USING btree (btree_test_func());
486+
COMMIT;
487+
DROP TABLE btree_test_expr;
488+
DROP FUNCTION btree_test_func();
479489
-- Test case of ALTER INDEX with abuse of column names for indexes.
480490
-- This grammar is not officially supported, but the parser allows it.
481491
CREATE INDEX btree_tall_idx2 ON btree_tall_tbl (id);

‎src/test/regress/sql/brin.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,17 @@ SELECT * FROM brintest_3 WHERE b < '0';
510510
DROP TABLE brintest_3;
511511
RESET enable_seqscan;
512512

513+
-- test parallel build with immutable function.
514+
CREATE TABLE brintest_expr (n int);
515+
CREATE FUNCTION brintest_func() RETURNS int LANGUAGE sql IMMUTABLE RETURN 0;
516+
BEGIN;
517+
SET LOCAL min_parallel_table_scan_size = 0;
518+
SET LOCAL max_parallel_maintenance_workers = 4;
519+
CREATE INDEX brintest_expr_idx ON brintest_expr USING brin (brintest_func());
520+
COMMIT;
521+
DROP TABLE brintest_expr;
522+
DROP FUNCTION brintest_func();
523+
513524
-- test an unlogged table, mostly to get coverage of brinbuildempty
514525
CREATE UNLOGGED TABLE brintest_unlogged (n numrange);
515526
CREATE INDEX brinidx_unlogged ON brintest_unlogged USING brin (n);

‎src/test/regress/sql/btree_index.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,17 @@ INSERT INTO delete_test_table SELECT i, 1, 2, 3 FROM generate_series(1,1000) i;
272272
-- Test unsupported btree opclass parameters
273273
create index on btree_tall_tbl (id int4_ops(foo=1));
274274

275+
-- test parallel build with immutable function.
276+
CREATE TABLE btree_test_expr (n int);
277+
CREATE FUNCTION btree_test_func() RETURNS int LANGUAGE sql IMMUTABLE RETURN 0;
278+
BEGIN;
279+
SET LOCAL min_parallel_table_scan_size = 0;
280+
SET LOCAL max_parallel_maintenance_workers = 4;
281+
CREATE INDEX btree_test_expr_idx ON btree_test_expr USING btree (btree_test_func());
282+
COMMIT;
283+
DROP TABLE btree_test_expr;
284+
DROP FUNCTION btree_test_func();
285+
275286
-- Test case of ALTER INDEX with abuse of column names for indexes.
276287
-- This grammar is not officially supported, but the parser allows it.
277288
CREATE INDEX btree_tall_idx2 ON btree_tall_tbl (id);

0 commit comments

Comments
 (0)