From 3c80e96dffd4df7f66fffa5f265cbd87becb7ef5 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 30 Apr 2021 14:46:42 +1200 Subject: [PATCH] Adjust EXPLAIN output for parallel Result Cache plans Here we adjust the EXPLAIN ANALYZE output for Result Cache so that we don't show any Result Cache stats for parallel workers who don't contribute anything to Result Cache plan nodes. I originally had ideas that workers who don't help could still have their Result Cache stats displayed. The idea with that was so that I could write some parallel Result Cache regression tests that show the EXPLAIN ANALYZE output. However, I realized a little too late that such tests would just not be possible to have run in a stable way on the buildfarm. With that knowledge, before 9eacee2e6 went in, I had removed all of the tests that were showing the EXPLAIN ANALYZE output of a parallel Result Cache plan, however, I forgot to put back the code that adjusts the EXPLAIN output to hide the Result Cache stats for parallel workers who were not fast enough to help out before query execution was over. All other nodes behave this way and so should Result Cache. Additionally, with this change, it now seems safe enough to remove the SET force_parallel_mode = off that I had added to the regression tests. Also, perform some cleanup in the partition_prune tests. I had adjusted the explain_parallel_append() function to sanitize the Result Cache EXPLAIN ANALYZE output. However, since I didn't actually include any parallel Result Cache tests that show their EXPLAIN ANALYZE output, that code does nothing and can be removed. In passing, move the setting of memPeakKb into the scope where it's used. Reported-by: Amit Khandekar Author: David Rowley, Amit Khandekar Discussion: https://postgr.es/m/CAJ3gD9d8SkfY95GpM1zmsOtX2-Ogx5q-WLsf8f0ykEb0hCRK3w@mail.gmail.com --- src/backend/commands/explain.c | 25 ++++++++++++------- src/test/regress/expected/partition_prune.out | 3 --- src/test/regress/expected/resultcache.out | 4 --- src/test/regress/sql/partition_prune.sql | 3 --- src/test/regress/sql/resultcache.sql | 5 +--- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 3d5198e2345..8ab7bca866b 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3144,17 +3144,17 @@ show_resultcache_info(ResultCacheState *rcstate, List *ancestors, if (!es->analyze) return; - /* - * mem_peak is only set when we freed memory, so we must use mem_used when - * mem_peak is 0. - */ - if (rcstate->stats.mem_peak > 0) - memPeakKb = (rcstate->stats.mem_peak + 1023) / 1024; - else - memPeakKb = (rcstate->mem_used + 1023) / 1024; - if (rcstate->stats.cache_misses > 0) { + /* + * mem_peak is only set when we freed memory, so we must use mem_used + * when mem_peak is 0. + */ + if (rcstate->stats.mem_peak > 0) + memPeakKb = (rcstate->stats.mem_peak + 1023) / 1024; + else + memPeakKb = (rcstate->mem_used + 1023) / 1024; + if (es->format != EXPLAIN_FORMAT_TEXT) { ExplainPropertyInteger("Cache Hits", NULL, rcstate->stats.cache_hits, es); @@ -3186,6 +3186,13 @@ show_resultcache_info(ResultCacheState *rcstate, List *ancestors, si = &rcstate->shared_info->sinstrument[n]; + /* + * Skip workers that didn't do any work. We needn't bother checking + * for cache hits as a miss will always occur before a cache hit. + */ + if (si->cache_misses == 0) + continue; + if (es->workers_state) ExplainOpenWorker(n, es); diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 1a7149bfd57..2c62e4a7a60 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -1958,9 +1958,6 @@ begin ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N'); ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N'); ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N'); - ln := regexp_replace(ln, 'Hits: \d+', 'Hits: N'); - ln := regexp_replace(ln, 'Misses: \d+', 'Misses: N'); - ln := regexp_replace(ln, 'Memory Usage: \d+', 'Memory Usage: N'); return next ln; end loop; end; diff --git a/src/test/regress/expected/resultcache.out b/src/test/regress/expected/resultcache.out index 7f4bf95f3fe..5b5dd6838e0 100644 --- a/src/test/regress/expected/resultcache.out +++ b/src/test/regress/expected/resultcache.out @@ -31,9 +31,6 @@ $$; -- Ensure we get a result cache on the inner side of the nested loop SET enable_hashjoin TO off; SET enable_bitmapscan TO off; --- force_parallel_mode = regress can cause some instability in EXPLAIN ANALYZE --- output, so let's ensure that we turn it off. -SET force_parallel_mode TO off; SELECT explain_resultcache(' SELECT COUNT(*),AVG(t1.unique1) FROM tenk1 t1 INNER JOIN tenk1 t2 ON t1.unique1 = t2.twenty @@ -118,7 +115,6 @@ WHERE t2.unique1 < 1200;', true); RESET enable_mergejoin; RESET work_mem; -RESET force_parallel_mode; RESET enable_bitmapscan; RESET enable_hashjoin; -- Test parallel plans with Result Cache. diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql index 247264f93b7..16c8dc5f1fa 100644 --- a/src/test/regress/sql/partition_prune.sql +++ b/src/test/regress/sql/partition_prune.sql @@ -464,9 +464,6 @@ begin ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N'); ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N'); ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N'); - ln := regexp_replace(ln, 'Hits: \d+', 'Hits: N'); - ln := regexp_replace(ln, 'Misses: \d+', 'Misses: N'); - ln := regexp_replace(ln, 'Memory Usage: \d+', 'Memory Usage: N'); return next ln; end loop; end; diff --git a/src/test/regress/sql/resultcache.sql b/src/test/regress/sql/resultcache.sql index 3fede90b006..43a70d56a51 100644 --- a/src/test/regress/sql/resultcache.sql +++ b/src/test/regress/sql/resultcache.sql @@ -33,9 +33,7 @@ $$; -- Ensure we get a result cache on the inner side of the nested loop SET enable_hashjoin TO off; SET enable_bitmapscan TO off; --- force_parallel_mode = regress can cause some instability in EXPLAIN ANALYZE --- output, so let's ensure that we turn it off. -SET force_parallel_mode TO off; + SELECT explain_resultcache(' SELECT COUNT(*),AVG(t1.unique1) FROM tenk1 t1 INNER JOIN tenk1 t2 ON t1.unique1 = t2.twenty @@ -69,7 +67,6 @@ INNER JOIN tenk1 t2 ON t1.unique1 = t2.thousand WHERE t2.unique1 < 1200;', true); RESET enable_mergejoin; RESET work_mem; -RESET force_parallel_mode; RESET enable_bitmapscan; RESET enable_hashjoin;