From: | Alena Rybakina <lena(dot)ribackina(at)yandex(dot)ru> |
---|---|
To: | Donghang Lin <donghanglin(at)gmail(dot)com> |
Cc: | David Geier <geidav(dot)pg(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, hlinnaka(at)iki(dot)fi |
Subject: | Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE |
Date: | 2024-04-08 21:16:56 |
Message-ID: | 13bd913f-94b6-43cf-b849-4d762e5297d8@yandex.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi! Thank you for your work on this issue!
Your patch required a little revision. I did this and attached the patch.
Also, I think you should add some clarification to the comments about
printing 'exact' and 'loosy' pages in show_hashagg_info function, which
you get from planstate->stats, whereas previously it was output only
from planstate. Perhaps it is enough to mention this in the comment to
the commit.
I mean this place:
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 926d70afaf..02251994c6 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3467,26 +3467,57 @@ show_hashagg_info(AggState *aggstate,
ExplainState *es)
static void
show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
{
+ Assert(es->analyze);
+
if (es->format != EXPLAIN_FORMAT_TEXT)
{
ExplainPropertyInteger("Exact Heap Blocks", NULL,
- planstate->exact_pages, es);
+ planstate->stats.exact_pages, es);
ExplainPropertyInteger("Lossy Heap Blocks", NULL,
- planstate->lossy_pages, es);
+ planstate->stats.lossy_pages, es);
}
else
{
- if (planstate->exact_pages > 0 || planstate->lossy_pages > 0)
+ if (planstate->stats.exact_pages > 0 ||
planstate->stats.lossy_pages > 0)
{
ExplainIndentText(es);
appendStringInfoString(es->str, "Heap Blocks:");
- if (planstate->exact_pages > 0)
- appendStringInfo(es->str, " exact=%ld",
planstate->exact_pages);
- if (planstate->lossy_pages > 0)
- appendStringInfo(es->str, " lossy=%ld",
planstate->lossy_pages);
+ if (planstate->stats.exact_pages > 0)
+ appendStringInfo(es->str, " exact=%ld",
planstate->stats.exact_pages);
+ if (planstate->stats.lossy_pages > 0)
+ appendStringInfo(es->str, " lossy=%ld",
planstate->stats.lossy_pages);
appendStringInfoChar(es->str, '\n');
}
}
+
+ if (planstate->pstate != NULL)
+ {
+ for (int n = 0; n < planstate->sinstrument->num_workers; n++)
+ {
+ BitmapHeapScanInstrumentation *si =
&planstate->sinstrument->sinstrument[n];
+
+ if (si->exact_pages == 0 && si->lossy_pages == 0)
+ continue;
+
+ if (es->workers_state)
+ ExplainOpenWorker(n, es);
+
+ if (es->format == EXPLAIN_FORMAT_TEXT)
+ {
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Heap Blocks: exact=%ld
lossy=%ld\n",
+ si->exact_pages, si->lossy_pages);
+ }
+ else
+ {
+ ExplainPropertyInteger("Exact Heap Blocks", NULL,
si->exact_pages, es);
+ ExplainPropertyInteger("Lossy Heap Blocks", NULL,
si->lossy_pages, es);
+ }
+
+ if (es->workers_state)
+ ExplainCloseWorker(n, es);
+ }
+ }
}
I suggest some code refactoring (diff.diff.no-cfbot file) that allows
you to improve your code.
--
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment | Content-Type | Size |
---|---|---|
diff.diff.no-cfbot | text/plain | 1.0 KB |
v6-0001-Parallel-Bitmap-Heap-Scan-reports-per-worker-stats.patch | text/x-patch | 11.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2024-04-08 21:35:00 | Re: post-freeze damage control |
Previous Message | Alexander Korotkov | 2024-04-08 20:49:46 | Re: Table AM Interface Enhancements |