Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit af52770

Browse files
committed
Adjust design of per-worker parallel seqscan data struct
The design of the data structures which allow storage of the per-worker memory during parallel seq scans were not ideal. The work done in 56788d2 required an additional data structure to allow workers to remember the range of pages that had been allocated to them for processing during a parallel seqscan. That commit added a void pointer field to TableScanDescData to allow heapam to store the per-worker allocation information. However putting the field there made very little sense given that we have AM specific structs for that, e.g. HeapScanDescData. Here we remove the void pointer field from TableScanDescData and add a dedicated field for this purpose to HeapScanDescData. Previously we also allocated memory for this parallel per-worker data for all scans, regardless if it was a parallel scan or not. This was just a wasted allocation for non-parallel scans, so here we make the allocation conditional on the scan being parallel. Also, add previously missing pfree() to free the per-worker data in heap_endscan(). Reported-by: Andres Freund Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20210317023101.anvejcfotwka6gaa@alap3.anarazel.de
1 parent 6d7a6fe commit af52770

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed

src/backend/access/heap/heapam.c

+16-6
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ heapgettup(HeapScanDesc scan,
540540
ParallelBlockTableScanDesc pbscan =
541541
(ParallelBlockTableScanDesc) scan->rs_base.rs_parallel;
542542
ParallelBlockTableScanWorker pbscanwork =
543-
(ParallelBlockTableScanWorker) scan->rs_base.rs_private;
543+
scan->rs_parallelworkerdata;
544544

545545
table_block_parallelscan_startblock_init(scan->rs_base.rs_rd,
546546
pbscanwork, pbscan);
@@ -748,7 +748,7 @@ heapgettup(HeapScanDesc scan,
748748
ParallelBlockTableScanDesc pbscan =
749749
(ParallelBlockTableScanDesc) scan->rs_base.rs_parallel;
750750
ParallelBlockTableScanWorker pbscanwork =
751-
(ParallelBlockTableScanWorker) scan->rs_base.rs_private;
751+
scan->rs_parallelworkerdata;
752752

753753
page = table_block_parallelscan_nextpage(scan->rs_base.rs_rd,
754754
pbscanwork, pbscan);
@@ -864,7 +864,7 @@ heapgettup_pagemode(HeapScanDesc scan,
864864
ParallelBlockTableScanDesc pbscan =
865865
(ParallelBlockTableScanDesc) scan->rs_base.rs_parallel;
866866
ParallelBlockTableScanWorker pbscanwork =
867-
(ParallelBlockTableScanWorker) scan->rs_base.rs_private;
867+
scan->rs_parallelworkerdata;
868868

869869
table_block_parallelscan_startblock_init(scan->rs_base.rs_rd,
870870
pbscanwork, pbscan);
@@ -1057,7 +1057,7 @@ heapgettup_pagemode(HeapScanDesc scan,
10571057
ParallelBlockTableScanDesc pbscan =
10581058
(ParallelBlockTableScanDesc) scan->rs_base.rs_parallel;
10591059
ParallelBlockTableScanWorker pbscanwork =
1060-
(ParallelBlockTableScanWorker) scan->rs_base.rs_private;
1060+
scan->rs_parallelworkerdata;
10611061

10621062
page = table_block_parallelscan_nextpage(scan->rs_base.rs_rd,
10631063
pbscanwork, pbscan);
@@ -1194,8 +1194,6 @@ heap_beginscan(Relation relation, Snapshot snapshot,
11941194
scan->rs_base.rs_nkeys = nkeys;
11951195
scan->rs_base.rs_flags = flags;
11961196
scan->rs_base.rs_parallel = parallel_scan;
1197-
scan->rs_base.rs_private =
1198-
palloc(sizeof(ParallelBlockTableScanWorkerData));
11991197
scan->rs_strategy = NULL; /* set in initscan */
12001198

12011199
/*
@@ -1231,6 +1229,15 @@ heap_beginscan(Relation relation, Snapshot snapshot,
12311229
/* we only need to set this up once */
12321230
scan->rs_ctup.t_tableOid = RelationGetRelid(relation);
12331231

1232+
/*
1233+
* Allocate memory to keep track of page allocation for parallel workers
1234+
* when doing a parallel scan.
1235+
*/
1236+
if (parallel_scan != NULL)
1237+
scan->rs_parallelworkerdata = palloc(sizeof(ParallelBlockTableScanWorkerData));
1238+
else
1239+
scan->rs_parallelworkerdata = NULL;
1240+
12341241
/*
12351242
* we do this here instead of in initscan() because heap_rescan also calls
12361243
* initscan() and we don't want to allocate memory again
@@ -1306,6 +1313,9 @@ heap_endscan(TableScanDesc sscan)
13061313
if (scan->rs_strategy != NULL)
13071314
FreeAccessStrategy(scan->rs_strategy);
13081315

1316+
if (scan->rs_parallelworkerdata != NULL)
1317+
pfree(scan->rs_parallelworkerdata);
1318+
13091319
if (scan->rs_base.rs_flags & SO_TEMP_SNAPSHOT)
13101320
UnregisterSnapshot(scan->rs_base.rs_snapshot);
13111321

src/include/access/heapam.h

+6
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ typedef struct HeapScanDescData
6565

6666
HeapTupleData rs_ctup; /* current tuple in scan, if any */
6767

68+
/*
69+
* For parallel scans to store page allocation data. NULL when not
70+
* performing a parallel scan.
71+
*/
72+
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
73+
6874
/* these fields only used in page-at-a-time mode and for bitmap scans */
6975
int rs_cindex; /* current tuple's index in vistuples */
7076
int rs_ntuples; /* number of visible tuples on page */

src/include/access/relscan.h

-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ typedef struct TableScanDescData
4646
*/
4747
uint32 rs_flags;
4848

49-
void *rs_private; /* per-worker private memory for AM to use */
5049
struct ParallelTableScanDescData *rs_parallel; /* parallel scan
5150
* information */
5251
} TableScanDescData;

0 commit comments

Comments
 (0)