Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE

Alena Rybakina <lena.ribackina@yandex.ru>

From: Alena Rybakina <lena.ribackina@yandex.ru>
To: Donghang Lin <donghanglin@gmail.com>
Cc: David Geier <geidav.pg@gmail.com>, Melanie Plageman <melanieplageman@gmail.com>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Dilip Kumar <dilipbalaut@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, hlinnaka@iki.fi
Date: 2024-04-08T21:16:56Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Show Parallel Bitmap Heap Scan worker stats in EXPLAIN ANALYZE

  2. Widen lossy and exact page counters for Bitmap Heap Scan

  3. Remove redundant snapshot copying from parallel leader to workers

Attachments

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