Fix Heap Blocks accumulation for Parallel Bitmap Heap Scan - Mailing list pgsql-hackers

From Lukas Fittl
Subject Fix Heap Blocks accumulation for Parallel Bitmap Heap Scan
Date
Msg-id CAP53PkxRrRKLXECGNFTVOtUusBoWLutZiPfnbejX40ocLuFMQA@mail.gmail.com
Whole thread Raw
Responses Re: Fix Heap Blocks accumulation for Parallel Bitmap Heap Scan
List pgsql-hackers
Hi,

In another thread [0], Andres noted that Parallel Bitmap Heap Scan and
Parallel Index-Only Scan don't have any test coverage for EXPLAIN
(ANALYZE):

parallel BHS is not covered:
https://coverage.postgresql.org/src/backend/executor/nodeBitmapHeapscan.c.gcov.html#L536
parallel IOS is not covered:
https://coverage.postgresql.org/src/backend/executor/nodeIndexonlyscan.c.gcov.html#L430

In the process of adding that coverage, I found a bug.

This is slightly different than the bug that Tomas and Melanie posted
about on an adjacent thread [1], so starting a new one:

When secondary instrumentation data (that's not WalUsage or
BufferUsage) gets handled by parallel workers, we need to do a lot of
special handling. On top of making sure we copy the information, we
also need to teach explain.c to aggregate the per-worker node
instrumentation, like done for index searches in
show_indexsearches_info:

static void
show_indexsearches_info(PlanState *planstate, ExplainState *es)
{
    Plan       *plan = planstate->plan;
    SharedIndexScanInstrumentation *SharedInfo = NULL;
    uint64        nsearches = 0;

    if (!es->analyze)
        return;
...
    /* Next get the sum of the counters set within each and every process */
    if (SharedInfo)
    {
        for (int i = 0; i < SharedInfo->num_workers; ++i)
        {
            IndexScanInstrumentation *winstrument = &SharedInfo->winstrument[i];

            nsearches += winstrument->nsearches;
        }
    }

    ExplainPropertyUInteger("Index Searches", NULL, nsearches, es);
}

Parallel Bitmap Heap Scans were missing such handling in
show_tidbitmap_info, causing the information shown on the plan to only
reflect the Heap Blocks of the leader, not that of the parallel
workers. I think this is inconsistent, and should be fixed.

See attached a patch that fixes that in show_tidbitmap_info and adds
test coverage. See also attached a second patch that adds missing
EXPLAIN ANALYZE test coverage for Parallel Index Only Scans.

Thanks,
Lukas

[0]: https://www.postgresql.org/message-id/57biou6l65r7gr4nunoe6lignz2x6m3w45gihoypaez4pc46di@txj3bakhj66l
[1]:
https://www.postgresql.org/message-id/flat/dbd45d67-8fb9-464f-b3ed-6fe185f8c8c9%40vondra.me#8e15b22526b969c8b730c065100aab72

--
Lukas Fittl

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: CREATE SCHEMA ... CREATE DOMAIN support
Next
From: Alvaro Herrera
Date:
Subject: Re: Adding REPACK [concurrently]