Re: Custom explain options

On 30/11/2023 22:40, Konstantin Knizhnik wrote:
> 
> 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.

I got it. But having a substantial number of extensions in support, I 
think the extension part of instrumentation could have advantages and be 
worth elaborating on.

> 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.
I see next basic instruments in the code:
- Instrumentation (which should be named NodeInstrumentation)
- MemoizeInstrumentation
- JitInstrumentation
- AggregateInstrumentation
- HashInstrumentation
- TuplesortInstrumentation

As a variant, extensibility can be designed with parent 
'AbstractInstrumentation' node, containing node type and link to 
extensible part. sizeof(Instr) calls should be replaced with the 
getInstrSize() call - not so much places in the code; memcpy() also can 
be replaced with the copy_instr() routine.

-- 
regards,
Andrei Lepikhov
Postgres Professional