From 5aa1c966f73aedd048bd2f1f847cd37a355c8add Mon Sep 17 00:00:00 2001 From: Yao Wang Date: Fri, 18 Sep 2026 16:11:17 +0300 Subject: [PATCH 1/4] Report work_mem wanted by a sort that spilled to disk EXPLAIN ANALYZE prints "Memory wanted": how much memory an operator needed to finish without writing to disk. Hash aggregation and hash join fill it in, the sort does not: the code that did it was lost when tuplesort was rewritten, and the call that hands the sort its Instrumentation was left disabled under a GPDB_12_MERGE_FIXME. So a query whose sort spilled gigabytes was advised to use less memory than it already had. This is a backport of the Greenplum 7 fix, greenplum-db/gpdb@fe0a9ddbdd ("re-implement explain analyze related code in tuplesort", #13590). It counts the rows the sort was given and how much memory those rows took when they were written out, and once the sort has spilled reports what it would have needed to hold them in memory: the rows themselves plus the array that points at them. Changes against the original: - PostgreSQL 14 sets up the per-sort state in tuplesort_begin_batch(), so the counters are cleared there, next to growmemtuples, where the original put them. - The size of the array is computed in 64 bits; 1 << my_log2(n) overflows an int once a sort is given more than 2^31 rows. - Without an Instrumentation, tuplesort_get_stats() keeps reporting the peak of the sort's memory context, as it did before. - The original also reported the sort's memory as the node's executor memory (execmemused), which adds an "Executor Memory" line under every sort in EXPLAIN ANALYZE. That is a separate change and is left out. A sort of 100k rows with statement_mem = '2MB': Sort Method: external merge Disk: 21696kB Memory wanted: 10885kB (nothing was printed before) Backported-by: Alena Rybakina --- src/backend/executor/nodeSort.c | 11 +-- src/backend/utils/sort/tuplesort.c | 84 ++++++++++++++++++- src/include/utils/tuplesort.h | 6 ++ src/test/regress/expected/explain_format.out | 33 ++++++++ .../expected/explain_format_optimizer.out | 33 ++++++++ src/test/regress/sql/explain_format.sql | 31 +++++++ 6 files changed, 190 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c index dcb4ccf4e20..cf1fdb396a8 100644 --- a/src/backend/executor/nodeSort.c +++ b/src/backend/executor/nodeSort.c @@ -122,13 +122,10 @@ ExecSort(PlanState *pstate) /* CDB */ /* If EXPLAIN ANALYZE, share our Instrumentation object with sort. */ - /* GPDB_12_MERGE_FIXME: broken */ -#if 0 if (node->ss.ps.instrument && node->ss.ps.instrument->need_cdb) tuplesort_set_instrument(tuplesortstate, node->ss.ps.instrument, node->ss.ps.cdbexplainbuf); -#endif /* * Scan the subplan and feed all the tuples to tuplesort. */ @@ -444,8 +441,8 @@ ExecSortExplainEnd(PlanState *planstate, struct StringInfoData *buf) if (sortstate->tuplesortstate) { - tuplesort_get_stats(sortstate->tuplesortstate, - &sortstate->sortstats); + tuplesort_finalize_stats(sortstate->tuplesortstate, + &sortstate->sortstats); if (planstate->instrument) { @@ -470,8 +467,8 @@ ExecEagerFreeSort(SortState *node) * Save stats like in ExecSortExplainEnd, so that we can display * them later in EXPLAIN ANALYZE. */ - tuplesort_get_stats(node->tuplesortstate, - &node->sortstats); + tuplesort_finalize_stats(node->tuplesortstate, + &node->sortstats); if (node->ss.ps.instrument) { node->ss.ps.instrument->workfileCreated = (node->sortstats.spaceType == SORT_SPACE_TYPE_DISK); diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 5a5415d4452..941fcaa67a6 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -113,6 +113,7 @@ #include "utils/rel.h" #include "utils/sortsupport.h" #include "utils/tuplesort.h" +#include "utils/dynahash.h" #include "utils/faultinjector.h" @@ -314,6 +315,7 @@ struct Tuplesortstate int memtupcount; /* number of tuples currently present */ int memtupsize; /* allocated length of memtuples array */ bool growmemtuples; /* memtuples' growth still underway? */ + int64 totalNumTuples; /* count of all input tuples */ /*CDB*/ /* * Memory for tuples is sometimes allocated using a simple slab allocator, @@ -476,6 +478,13 @@ struct Tuplesortstate /* we need typelen in order to know how to copy the Datums. */ int datumTypeLen; + /* + * CDB: EXPLAIN ANALYZE reporting interface and statistics. + */ + struct Instrumentation *instrument; + struct StringInfoData *explainbuf; + uint64 spilledBytes; + /* * Resource snapshot for time of sort start. */ @@ -868,6 +877,8 @@ tuplesort_begin_batch(Tuplesortstate *state) * see comments in grow_memtuples(). */ state->growmemtuples = true; + state->totalNumTuples = 0; /*CDB*/ + state->spilledBytes = 0; /*CDB*/ state->slabAllocatorUsed = false; if (state->memtuples != NULL && state->memtupsize != INITIAL_MEMTUPSIZE) { @@ -1894,6 +1905,8 @@ puttuple_common(Tuplesortstate *state, SortTuple *tuple) { Assert(!LEADER(state)); + state->totalNumTuples++; + switch (state->status) { case TSS_INITIAL: @@ -2118,6 +2131,24 @@ tuplesort_performsort(Tuplesortstate *state) * Note that mergeruns sets the correct state->status. */ dumptuples(state, true); + + /* CDB: How much work_mem would be enough for in-memory sort? */ + if (state->instrument && state->instrument->need_cdb) + { + /* + * The workmemwanted is summed up of the following: + * (1) metadata: Tuplesortstate, tuple array + * (2) the total bytes for all tuples. + */ + int64 workmemwanted = + sizeof(Tuplesortstate) + + ((uint64) 1 << my_log2(state->totalNumTuples)) * sizeof(SortTuple) + + state->spilledBytes; + + state->instrument->workmemwanted = + Max(state->instrument->workmemwanted, workmemwanted); + } + mergeruns(state); state->eof_reached = false; state->markpos_block = 0L; @@ -3223,6 +3254,7 @@ dumptuples(Tuplesortstate *state, bool alltuples) { int memtupwrite; int i; + long prevAvailMem = state->availMem; /* * Nothing to do if we still fit in available memory and have array slots, @@ -3340,6 +3372,12 @@ dumptuples(Tuplesortstate *state, bool alltuples) pg_rusage_show(&state->ru_start)); #endif + /* CDB: Accumulate total size of spilled tuples. */ + if (state->availMem > prevAvailMem) + { + state->spilledBytes += state->availMem - prevAvailMem; + } + if (!alltuples) selectnewtape(state); } @@ -3471,7 +3509,10 @@ tuplesort_get_stats(Tuplesortstate *state, else stats->spaceType = SORT_SPACE_TYPE_MEMORY; stats->spaceUsed = (state->maxSpace + 1023) / 1024; - stats->workmemused = MemoryContextGetPeakSpace(state->sortcontext); + if (state->instrument) + stats->workmemused = state->instrument->workmemused; + else + stats->workmemused = MemoryContextGetPeakSpace(state->sortcontext); switch (state->maxSpaceStatus) { @@ -4877,3 +4918,44 @@ free_sort_tuple(Tuplesortstate *state, SortTuple *stup) stup->tuple = NULL; } } + +/* + * tuplesort_set_instrument + * + * May be called after tuplesort_begin_xxx() to enable reporting of + * statistics and events for EXPLAIN ANALYZE. + * + * The 'instr' and 'explainbuf' ptrs are retained in the 'state' object for + * possible use anytime during the sort, up to and including tuplesort_end(). + * The caller must ensure that the referenced objects remain allocated and + * valid for the life of the Tuplesortstate object; or if they are to be + * freed early, disconnect them by calling again with NULL pointers. + */ +void +tuplesort_set_instrument(Tuplesortstate *state, + struct Instrumentation *instrument, + struct StringInfoData *explainbuf) +{ + state->instrument = instrument; + state->explainbuf = explainbuf; +} + +/* + * tuplesort_finalize_stats + * + * Finalize the EXPLAIN ANALYZE stats. + */ +void +tuplesort_finalize_stats(Tuplesortstate *state, + TuplesortInstrumentation *stats) +{ + if (state->instrument) + { + double workmemused; + + workmemused = MemoryContextGetPeakSpace(state->sortcontext); + if (state->instrument->workmemused < workmemused) + state->instrument->workmemused = workmemused; + } + tuplesort_get_stats(state, stats); +} diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index c1143f9a8bb..e0cab6194d9 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -263,6 +263,8 @@ extern void tuplesort_reset(Tuplesortstate *state); extern void tuplesort_get_stats(Tuplesortstate *state, TuplesortInstrumentation *stats); +extern void tuplesort_finalize_stats(Tuplesortstate *state, + TuplesortInstrumentation *stats); extern const char *tuplesort_method_name(TuplesortMethod m); extern const char *tuplesort_space_type_name(TuplesortSpaceType t); @@ -273,6 +275,10 @@ extern void tuplesort_initialize_shared(Sharedsort *shared, int nWorkers, dsm_segment *seg); extern void tuplesort_attach_shared(Sharedsort *shared, dsm_segment *seg); +extern void tuplesort_set_instrument(Tuplesortstate *state, + struct Instrumentation *instrument, + struct StringInfoData *explainbuf); + /* * These routines may only be called if randomAccess was specified 'true'. * Likewise, backwards scan in gettuple/getdatum is only allowed if diff --git a/src/test/regress/expected/explain_format.out b/src/test/regress/expected/explain_format.out index a57a1fd55fc..627521c441e 100644 --- a/src/test/regress/expected/explain_format.out +++ b/src/test/regress/expected/explain_format.out @@ -643,3 +643,36 @@ DROP TABLE jsonexplaintest; DROP TABLE test_src_tbl; DROP TABLE test_hashagg_spill; DROP TABLE test_hashagg_groupingsets; +-- A sort that spills to disk must be reflected in "Memory wanted". +CREATE TABLE memwanted_sort (id int, pad text) DISTRIBUTED BY (id); +INSERT INTO memwanted_sort SELECT g, repeat('x', 200) FROM generate_series(1, 100000) g; +ANALYZE memwanted_sort; +CREATE FUNCTION sort_spill_vs_wanted(query text, OUT spilled_kb bigint, OUT wanted_kb bigint) +LANGUAGE plpgsql AS $$ +DECLARE + ln text; + m text[]; +BEGIN + spilled_kb := 0; + wanted_kb := 0; + FOR ln IN EXECUTE 'EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) ' || query LOOP + m := regexp_match(ln, 'Sort Method:\s+external merge\s+Disk:\s+(\d+)kB'); + IF m IS NOT NULL THEN + spilled_kb := greatest(spilled_kb, m[1]::bigint); + END IF; + m := regexp_match(ln, 'Memory wanted:\s+(\d+)kB'); + IF m IS NOT NULL THEN + wanted_kb := m[1]::bigint; + END IF; + END LOOP; +END $$; +-- 2MB is not enough for the sort, so it spills and must ask for more. +SET statement_mem = '2MB'; +SELECT spilled_kb > 0 AS sort_spilled, wanted_kb > 2048 AS wants_more_than_given + FROM sort_spill_vs_wanted('SELECT * FROM memwanted_sort ORDER BY pad, id'); +sort_spilled|wants_more_than_given +t|t +(1 row) +RESET statement_mem; +DROP FUNCTION sort_spill_vs_wanted(text); +DROP TABLE memwanted_sort; diff --git a/src/test/regress/expected/explain_format_optimizer.out b/src/test/regress/expected/explain_format_optimizer.out index f778c3f2048..fbc2cec281f 100644 --- a/src/test/regress/expected/explain_format_optimizer.out +++ b/src/test/regress/expected/explain_format_optimizer.out @@ -588,3 +588,36 @@ DROP TABLE jsonexplaintest; DROP TABLE test_src_tbl; DROP TABLE test_hashagg_spill; DROP TABLE test_hashagg_groupingsets; +-- A sort that spills to disk must be reflected in "Memory wanted". +CREATE TABLE memwanted_sort (id int, pad text) DISTRIBUTED BY (id); +INSERT INTO memwanted_sort SELECT g, repeat('x', 200) FROM generate_series(1, 100000) g; +ANALYZE memwanted_sort; +CREATE FUNCTION sort_spill_vs_wanted(query text, OUT spilled_kb bigint, OUT wanted_kb bigint) +LANGUAGE plpgsql AS $$ +DECLARE + ln text; + m text[]; +BEGIN + spilled_kb := 0; + wanted_kb := 0; + FOR ln IN EXECUTE 'EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) ' || query LOOP + m := regexp_match(ln, 'Sort Method:\s+external merge\s+Disk:\s+(\d+)kB'); + IF m IS NOT NULL THEN + spilled_kb := greatest(spilled_kb, m[1]::bigint); + END IF; + m := regexp_match(ln, 'Memory wanted:\s+(\d+)kB'); + IF m IS NOT NULL THEN + wanted_kb := m[1]::bigint; + END IF; + END LOOP; +END $$; +-- 2MB is not enough for the sort, so it spills and must ask for more. +SET statement_mem = '2MB'; +SELECT spilled_kb > 0 AS sort_spilled, wanted_kb > 2048 AS wants_more_than_given + FROM sort_spill_vs_wanted('SELECT * FROM memwanted_sort ORDER BY pad, id'); +sort_spilled|wants_more_than_given +t|t +(1 row) +RESET statement_mem; +DROP FUNCTION sort_spill_vs_wanted(text); +DROP TABLE memwanted_sort; diff --git a/src/test/regress/sql/explain_format.sql b/src/test/regress/sql/explain_format.sql index 229fcdfe6fe..caa092bb427 100644 --- a/src/test/regress/sql/explain_format.sql +++ b/src/test/regress/sql/explain_format.sql @@ -152,3 +152,34 @@ DROP TABLE jsonexplaintest; DROP TABLE test_src_tbl; DROP TABLE test_hashagg_spill; DROP TABLE test_hashagg_groupingsets; + +-- A sort that spills to disk must be reflected in "Memory wanted". +CREATE TABLE memwanted_sort (id int, pad text) DISTRIBUTED BY (id); +INSERT INTO memwanted_sort SELECT g, repeat('x', 200) FROM generate_series(1, 100000) g; +ANALYZE memwanted_sort; +CREATE FUNCTION sort_spill_vs_wanted(query text, OUT spilled_kb bigint, OUT wanted_kb bigint) +LANGUAGE plpgsql AS $$ +DECLARE + ln text; + m text[]; +BEGIN + spilled_kb := 0; + wanted_kb := 0; + FOR ln IN EXECUTE 'EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) ' || query LOOP + m := regexp_match(ln, 'Sort Method:\s+external merge\s+Disk:\s+(\d+)kB'); + IF m IS NOT NULL THEN + spilled_kb := greatest(spilled_kb, m[1]::bigint); + END IF; + m := regexp_match(ln, 'Memory wanted:\s+(\d+)kB'); + IF m IS NOT NULL THEN + wanted_kb := m[1]::bigint; + END IF; + END LOOP; +END $$; +-- 2MB is not enough for the sort, so it spills and must ask for more. +SET statement_mem = '2MB'; +SELECT spilled_kb > 0 AS sort_spilled, wanted_kb > 2048 AS wants_more_than_given + FROM sort_spill_vs_wanted('SELECT * FROM memwanted_sort ORDER BY pad, id'); +RESET statement_mem; +DROP FUNCTION sort_spill_vs_wanted(text); +DROP TABLE memwanted_sort; From 1628b23bcd06e8b9ef06073337dc0cfb9b59010d Mon Sep 17 00:00:00 2001 From: Alena Rybakina Date: Fri, 18 Sep 2026 16:11:47 +0300 Subject: [PATCH 2/4] Report work_mem wanted by parallel sort workers In a parallel sort every worker sorts its own share of the rows, but only the leader's sort was connected to the node's Instrumentation, so "Memory wanted" ignored whatever the workers spilled. Keep the estimate in the sort state as well and hand it over through the statistics the workers already store in shared memory; the leader reports the largest of them. --- src/backend/executor/nodeSort.c | 19 +++++++++++++++++ src/backend/utils/sort/tuplesort.c | 34 ++++++++++++++++++------------ src/include/utils/tuplesort.h | 1 + 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c index cf1fdb396a8..4ffd448d203 100644 --- a/src/backend/executor/nodeSort.c +++ b/src/backend/executor/nodeSort.c @@ -576,4 +576,23 @@ ExecSortRetrieveInstrumentation(SortState *node) si = palloc(size); memcpy(si, node->shared_info, size); node->shared_info = si; + + /* + * GPDB: the workers sorted their own share of the rows, so let + * "Memory wanted" account for them too, not just for the leader's sort. + */ + if (node->ss.ps.instrument) + { + int n; + + for (n = 0; n < si->num_workers; n++) + { + if (si->sinstrument[n].sortMethod == SORT_TYPE_STILL_IN_PROGRESS) + continue; + + node->ss.ps.instrument->workmemwanted = + Max(node->ss.ps.instrument->workmemwanted, + si->sinstrument[n].workmemwanted); + } + } } diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 941fcaa67a6..4d7a00c138f 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -484,6 +484,7 @@ struct Tuplesortstate struct Instrumentation *instrument; struct StringInfoData *explainbuf; uint64 spilledBytes; + int64 workmemwanted; /* work_mem for an in-memory sort */ /* * Resource snapshot for time of sort start. @@ -879,6 +880,7 @@ tuplesort_begin_batch(Tuplesortstate *state) state->growmemtuples = true; state->totalNumTuples = 0; /*CDB*/ state->spilledBytes = 0; /*CDB*/ + state->workmemwanted = 0; /*CDB*/ state->slabAllocatorUsed = false; if (state->memtuples != NULL && state->memtupsize != INITIAL_MEMTUPSIZE) { @@ -2132,22 +2134,25 @@ tuplesort_performsort(Tuplesortstate *state) */ dumptuples(state, true); - /* CDB: How much work_mem would be enough for in-memory sort? */ - if (state->instrument && state->instrument->need_cdb) - { - /* - * The workmemwanted is summed up of the following: - * (1) metadata: Tuplesortstate, tuple array - * (2) the total bytes for all tuples. - */ - int64 workmemwanted = - sizeof(Tuplesortstate) + - ((uint64) 1 << my_log2(state->totalNumTuples)) * sizeof(SortTuple) + - state->spilledBytes; + /* + * CDB: How much work_mem would be enough for in-memory sort? + * + * The workmemwanted is summed up of the following: + * (1) metadata: Tuplesortstate, tuple array + * (2) the total bytes for all tuples. + * + * It is kept in the state as well, so that tuplesort_get_stats() + * can hand it over from a parallel worker, which has no + * Instrumentation of its own. + */ + state->workmemwanted = + sizeof(Tuplesortstate) + + ((uint64) 1 << my_log2(state->totalNumTuples)) * sizeof(SortTuple) + + state->spilledBytes; + if (state->instrument && state->instrument->need_cdb) state->instrument->workmemwanted = - Max(state->instrument->workmemwanted, workmemwanted); - } + Max(state->instrument->workmemwanted, state->workmemwanted); mergeruns(state); state->eof_reached = false; @@ -3513,6 +3518,7 @@ tuplesort_get_stats(Tuplesortstate *state, stats->workmemused = state->instrument->workmemused; else stats->workmemused = MemoryContextGetPeakSpace(state->sortcontext); + stats->workmemwanted = state->workmemwanted; switch (state->maxSpaceStatus) { diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index e0cab6194d9..e64391132b6 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -97,6 +97,7 @@ typedef struct TuplesortInstrumentation long spaceUsed; /* space consumption, in kB */ Size workmemused; + Size workmemwanted; /* GPDB: work_mem for an in-memory sort */ } TuplesortInstrumentation; From a236719657b11f45fb3ec3c61b95d197b7007df3 Mon Sep 17 00:00:00 2001 From: Alena Rybakina Date: Fri, 18 Sep 2026 13:12:09 +0300 Subject: [PATCH 3/4] Make Incremental Sort follow statement_mem Operators that need a lot of memory each get a share of statement_mem. Incremental Sort did not: it always sized its sorts from work_mem, and it was counted among the cheap operators that get 100kB, so it was also left out when the memory need of the whole statement was worked out. Raising statement_mem therefore did nothing for an Incremental Sort that spilled. 2M rows in five groups, statement_mem = '64MB': before: Sort Method: external merge Average Disk: 16102kB after: Sort Method: quicksort Average Memory: 41738kB Give the node its share, and count it as an operator that needs memory. --- src/backend/executor/nodeIncrementalSort.c | 4 ++-- src/backend/utils/resource_manager/memquota.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c index 7a1db175446..2dfb8890b27 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -313,7 +313,7 @@ switchToPresortedPrefixMode(PlanState *pstate) &(plannode->sort.sortOperators[nPresortedCols]), &(plannode->sort.collations[nPresortedCols]), &(plannode->sort.nullsFirst[nPresortedCols]), - work_mem, + PlanStateOperatorMemKB((PlanState *) node), NULL, false); node->prefixsort_state = prefixsort_state; @@ -614,7 +614,7 @@ ExecIncrementalSort(PlanState *pstate) plannode->sort.sortOperators, plannode->sort.collations, plannode->sort.nullsFirst, - work_mem, + PlanStateOperatorMemKB((PlanState *) node), NULL, false); node->fullsort_state = fullsort_state; diff --git a/src/backend/utils/resource_manager/memquota.c b/src/backend/utils/resource_manager/memquota.c index 1cf5f805df9..064b9211137 100644 --- a/src/backend/utils/resource_manager/memquota.c +++ b/src/backend/utils/resource_manager/memquota.c @@ -243,6 +243,7 @@ IsMemoryIntensiveOperator(Node *node, PlannedStmt *stmt) { case T_Material: case T_Sort: + case T_IncrementalSort: case T_ShareInputScan: case T_Hash: case T_BitmapIndexScan: From 8a9ade9760b4d0f24dd53f56743abca28d0ec387 Mon Sep 17 00:00:00 2001 From: Alena Rybakina Date: Fri, 18 Sep 2026 16:12:03 +0300 Subject: [PATCH 4/4] Show how much memory an Incremental Sort used and wanted Incremental Sort sorts the rows group by group, each group in a sort of its own, and none of those figures reached EXPLAIN ANALYZE. The plan showed the work files the node had written, but said nothing about the memory it took, nor about the memory it was short of: before: Executor memory: 2045K bytes avg x 3x(0) workers, ... after: Executor memory: ... Work_mem: 1622K bytes max, 42775K bytes wanted. Report the busiest group, in the leader and in parallel workers alike, and record that the node wrote work files. Each group is measured on its own, since the sort clears its counters whenever it starts the next one, so the advice does not grow with the number of groups. --- src/backend/executor/nodeIncrementalSort.c | 49 +++++++++++++++++-- src/include/nodes/execnodes.h | 2 + src/test/regress/expected/explain_format.out | 31 ++++++++++++ .../expected/explain_format_optimizer.out | 31 ++++++++++++ src/test/regress/sql/explain_format.sql | 31 ++++++++++++ 5 files changed, 141 insertions(+), 3 deletions(-) diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c index 2dfb8890b27..fd75bdbc37b 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -105,12 +105,14 @@ Assert(IsParallelWorker()); \ Assert(ParallelWorkerNumber <= (node)->shared_info->num_workers); \ instrumentSortedGroup(&(node)->shared_info->sinfo[ParallelWorkerNumber].groupName##GroupInfo, \ - (node)->groupName##_state); \ + (node)->groupName##_state, \ + (node)->ss.ps.instrument); \ } \ else \ { \ instrumentSortedGroup(&(node)->incsort_info.groupName##GroupInfo, \ - (node)->groupName##_state); \ + (node)->groupName##_state, \ + (node)->ss.ps.instrument); \ } \ } \ } while (0) @@ -126,7 +128,8 @@ */ static void instrumentSortedGroup(IncrementalSortGroupInfo *groupInfo, - Tuplesortstate *sortState) + Tuplesortstate *sortState, + Instrumentation *instr) { TuplesortInstrumentation sort_instr; @@ -151,6 +154,26 @@ instrumentSortedGroup(IncrementalSortGroupInfo *groupInfo, break; } + /* + * GPDB: remember how much memory the batches of this node used, and the + * largest work_mem any single batch would have needed to stay in memory, + * so that EXPLAIN ANALYZE can report "Memory used" and "Memory wanted". + */ + if ((int64) sort_instr.workmemused > groupInfo->maxWorkmemUsed) + groupInfo->maxWorkmemUsed = sort_instr.workmemused; + if ((int64) sort_instr.workmemwanted > groupInfo->maxWorkmemWanted) + groupInfo->maxWorkmemWanted = sort_instr.workmemwanted; + + if (instr != NULL) + { + if (instr->workmemused < groupInfo->maxWorkmemUsed) + instr->workmemused = groupInfo->maxWorkmemUsed; + if (instr->workmemwanted < groupInfo->maxWorkmemWanted) + instr->workmemwanted = groupInfo->maxWorkmemWanted; + if (sort_instr.spaceType == SORT_SPACE_TYPE_DISK) + instr->workfileCreated = true; + } + /* Track each sort method we've used. */ groupInfo->sortMethods |= sort_instr.sortMethod; } @@ -1253,4 +1276,24 @@ ExecIncrementalSortRetrieveInstrumentation(IncrementalSortState *node) si = palloc(size); memcpy(si, node->shared_info, size); node->shared_info = si; + + /* + * GPDB: the workers' memory figures are not aggregated by the generic + * instrumentation code, so fold them into this node's instrumentation. + */ + if (node->ss.ps.instrument != NULL) + { + for (int n = 0; n < si->num_workers; n++) + { + int64 used = Max(si->sinfo[n].fullsortGroupInfo.maxWorkmemUsed, + si->sinfo[n].prefixsortGroupInfo.maxWorkmemUsed); + int64 wanted = Max(si->sinfo[n].fullsortGroupInfo.maxWorkmemWanted, + si->sinfo[n].prefixsortGroupInfo.maxWorkmemWanted); + + if (node->ss.ps.instrument->workmemused < used) + node->ss.ps.instrument->workmemused = used; + if (node->ss.ps.instrument->workmemwanted < wanted) + node->ss.ps.instrument->workmemwanted = wanted; + } + } } diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 63a4b212bfe..5d9154bf773 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2649,6 +2649,8 @@ typedef struct IncrementalSortGroupInfo int64 totalDiskSpaceUsed; int64 maxMemorySpaceUsed; int64 totalMemorySpaceUsed; + int64 maxWorkmemUsed; /* CDB: work_mem actually used */ + int64 maxWorkmemWanted; /* CDB: work_mem to avoid scratch i/o */ bits32 sortMethods; /* bitmask of TuplesortMethod */ } IncrementalSortGroupInfo; diff --git a/src/test/regress/expected/explain_format.out b/src/test/regress/expected/explain_format.out index 627521c441e..244eccb8b4b 100644 --- a/src/test/regress/expected/explain_format.out +++ b/src/test/regress/expected/explain_format.out @@ -660,6 +660,11 @@ BEGIN IF m IS NOT NULL THEN spilled_kb := greatest(spilled_kb, m[1]::bigint); END IF; + -- an Incremental Sort reports the disk space of its batches instead + m := regexp_match(ln, 'Sort Method:\s+external merge\s+Average Disk:\s+(\d+)kB'); + IF m IS NOT NULL THEN + spilled_kb := greatest(spilled_kb, m[1]::bigint); + END IF; m := regexp_match(ln, 'Memory wanted:\s+(\d+)kB'); IF m IS NOT NULL THEN wanted_kb := m[1]::bigint; @@ -674,5 +679,31 @@ sort_spilled|wants_more_than_given t|t (1 row) RESET statement_mem; +-- An Incremental Sort sorts one group of rows at a time. Its advice has to +-- describe the largest group rather than the sum of all of them, and the node +-- has to use the memory the statement was given. +CREATE TABLE memwanted_incsort(id bigint, grp int, pad text) DISTRIBUTED BY (id); +INSERT INTO memwanted_incsort + SELECT g, g % 5, repeat(chr(97 + g % 26), 100) FROM generate_series(1, 120000) g; +CREATE INDEX ON memwanted_incsort (grp); +ANALYZE memwanted_incsort; +SET enable_incremental_sort = on; +SET enable_sort = off; +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SET statement_mem = '1MB'; +SELECT spilled_kb > 0 AS incsort_spilled, + wanted_kb > 1024 AS wants_more_than_given, + wanted_kb < 5 * spilled_kb AS wants_one_group_not_all + FROM sort_spill_vs_wanted('SELECT * FROM memwanted_incsort ORDER BY grp, pad'); +incsort_spilled|wants_more_than_given|wants_one_group_not_all +t|t|t +(1 row) +RESET statement_mem; +RESET enable_incremental_sort; +RESET enable_sort; +RESET enable_seqscan; +RESET enable_bitmapscan; DROP FUNCTION sort_spill_vs_wanted(text); DROP TABLE memwanted_sort; +DROP TABLE memwanted_incsort; diff --git a/src/test/regress/expected/explain_format_optimizer.out b/src/test/regress/expected/explain_format_optimizer.out index fbc2cec281f..dfa611a2f0d 100644 --- a/src/test/regress/expected/explain_format_optimizer.out +++ b/src/test/regress/expected/explain_format_optimizer.out @@ -605,6 +605,11 @@ BEGIN IF m IS NOT NULL THEN spilled_kb := greatest(spilled_kb, m[1]::bigint); END IF; + -- an Incremental Sort reports the disk space of its batches instead + m := regexp_match(ln, 'Sort Method:\s+external merge\s+Average Disk:\s+(\d+)kB'); + IF m IS NOT NULL THEN + spilled_kb := greatest(spilled_kb, m[1]::bigint); + END IF; m := regexp_match(ln, 'Memory wanted:\s+(\d+)kB'); IF m IS NOT NULL THEN wanted_kb := m[1]::bigint; @@ -619,5 +624,31 @@ sort_spilled|wants_more_than_given t|t (1 row) RESET statement_mem; +-- An Incremental Sort sorts one group of rows at a time. Its advice has to +-- describe the largest group rather than the sum of all of them, and the node +-- has to use the memory the statement was given. +CREATE TABLE memwanted_incsort(id bigint, grp int, pad text) DISTRIBUTED BY (id); +INSERT INTO memwanted_incsort + SELECT g, g % 5, repeat(chr(97 + g % 26), 100) FROM generate_series(1, 120000) g; +CREATE INDEX ON memwanted_incsort (grp); +ANALYZE memwanted_incsort; +SET enable_incremental_sort = on; +SET enable_sort = off; +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SET statement_mem = '1MB'; +SELECT spilled_kb > 0 AS incsort_spilled, + wanted_kb > 1024 AS wants_more_than_given, + wanted_kb < 5 * spilled_kb AS wants_one_group_not_all + FROM sort_spill_vs_wanted('SELECT * FROM memwanted_incsort ORDER BY grp, pad'); +incsort_spilled|wants_more_than_given|wants_one_group_not_all +t|t|t +(1 row) +RESET statement_mem; +RESET enable_incremental_sort; +RESET enable_sort; +RESET enable_seqscan; +RESET enable_bitmapscan; DROP FUNCTION sort_spill_vs_wanted(text); DROP TABLE memwanted_sort; +DROP TABLE memwanted_incsort; diff --git a/src/test/regress/sql/explain_format.sql b/src/test/regress/sql/explain_format.sql index caa092bb427..fd540e397a4 100644 --- a/src/test/regress/sql/explain_format.sql +++ b/src/test/regress/sql/explain_format.sql @@ -170,6 +170,11 @@ BEGIN IF m IS NOT NULL THEN spilled_kb := greatest(spilled_kb, m[1]::bigint); END IF; + -- an Incremental Sort reports the disk space of its batches instead + m := regexp_match(ln, 'Sort Method:\s+external merge\s+Average Disk:\s+(\d+)kB'); + IF m IS NOT NULL THEN + spilled_kb := greatest(spilled_kb, m[1]::bigint); + END IF; m := regexp_match(ln, 'Memory wanted:\s+(\d+)kB'); IF m IS NOT NULL THEN wanted_kb := m[1]::bigint; @@ -181,5 +186,31 @@ SET statement_mem = '2MB'; SELECT spilled_kb > 0 AS sort_spilled, wanted_kb > 2048 AS wants_more_than_given FROM sort_spill_vs_wanted('SELECT * FROM memwanted_sort ORDER BY pad, id'); RESET statement_mem; + +-- An Incremental Sort sorts one group of rows at a time. Its advice has to +-- describe the largest group rather than the sum of all of them, and the node +-- has to use the memory the statement was given. +CREATE TABLE memwanted_incsort(id bigint, grp int, pad text) DISTRIBUTED BY (id); +INSERT INTO memwanted_incsort + SELECT g, g % 5, repeat(chr(97 + g % 26), 100) FROM generate_series(1, 120000) g; +CREATE INDEX ON memwanted_incsort (grp); +ANALYZE memwanted_incsort; + +SET enable_incremental_sort = on; +SET enable_sort = off; +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SET statement_mem = '1MB'; +SELECT spilled_kb > 0 AS incsort_spilled, + wanted_kb > 1024 AS wants_more_than_given, + wanted_kb < 5 * spilled_kb AS wants_one_group_not_all + FROM sort_spill_vs_wanted('SELECT * FROM memwanted_incsort ORDER BY grp, pad'); +RESET statement_mem; +RESET enable_incremental_sort; +RESET enable_sort; +RESET enable_seqscan; +RESET enable_bitmapscan; + DROP FUNCTION sort_spill_vs_wanted(text); DROP TABLE memwanted_sort; +DROP TABLE memwanted_incsort;