Re: Custom explain options

On 30/11/2023 5:59 am, Andrei Lepikhov wrote:
> On 21/10/2023 19:16, Konstantin Knizhnik wrote:
>> EXPLAIN statement has a list of options (i.e. ANALYZE, BUFFERS,
>> COST,...) which help to provide useful details of query execution.
>> In Neon we have added PREFETCH option which shows information about
>> page prefetching during query execution (prefetching is more critical
>> for Neon
>> architecture because of separation of compute and storage, so it is
>> implemented not only for bitmap heap scan as in Vanilla Postgres, but
>> also for seqscan, indexscan and indexonly scan). Another possible
>> candidate  for explain options is local file cache (extra caching
>> layer above shared buffers which is used to somehow replace file
>> system cache in standalone Postgres).
>>
>> I think that it will be nice to have a generic mechanism which allows
>> extensions to add its own options to EXPLAIN.
>
> Generally, I welcome this idea: Extensions can already do a lot of
> work, and they should have a tool to report their state, not only into
> the log.
> But I think your approach needs to be elaborated. At first, it would
> be better to allow registering extended instruments for specific node
> types to avoid unneeded calls.
> Secondly, looking into the Instrumentation usage, I don't see the
> reason to limit the size: as I see everywhere it exists locally or in
> the DSA where its size is calculated on the fly. So, by registering an
> extended instrument, we can reserve a slot for the extension. The
> actual size of underlying data can be provided by the extension routine.
>
Thank you for review.

I agree that support of extended instruments is desired. I just tried to
minimize number of changes to make this patch smaller.

Concerning limiting instrumentation size, may be I missed something, but
I do not see any goo way to handle this:

```

./src/backend/executor/nodeMemoize.c1106:        si =
&node->shared_info->sinstrument[ParallelWorkerNumber];
./src/backend/executor/nodeAgg.c4322:        si =
&node->shared_info->sinstrument[ParallelWorkerNumber];
./src/backend/executor/nodeIncrementalSort.c107:
instrumentSortedGroup(&(node)->shared_info->sinfo[ParallelWorkerNumber].groupName##GroupInfo,
\
./src/backend/executor/execParallel.c808: InstrInit(&instrument[i],
estate->es_instrument);
./src/backend/executor/execParallel.c1052:
InstrAggNode(planstate->instrument, &instrument[n]);
./src/backend/executor/execParallel.c1306:
InstrAggNode(&instrument[ParallelWorkerNumber], planstate->instrument);
./src/backend/commands/explain.c1763:            Instrumentation
*instrument = &w->instrument[n];
./src/backend/commands/explain.c2168:            Instrumentation
*instrument = &w->instrument[n];
```

In all this cases we are using array of `Instrumentation` and if it
contains varying part, then it is not clear where to place it.
Yes, there is also code which serialize and sends instrumentations
between worker processes  and I have updated this code in my PR to send
actual amount of custom instrumentation data. But it can not help with
the cases above.