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.