On Mon, Oct 31, 2016 at 11:33 AM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> Hello,
>
> The attached patch implements the suggestion by Amit before.
>
> What I'm motivated is to collect extra run-time statistics specific
> to a particular ForeignScan/CustomScan, not only the standard
> Instrumentation; like DMA transfer rate or execution time of GPU
> kernels in my case.
>
> Per-node DSM toc is one of the best way to return run-time statistics
> to the master backend, because FDW/CSP can assign arbitrary length of
> the region according to its needs. It is quite easy to require.
> However, one problem is, the per-node DSM toc is already released when
> ExecEndNode() is called on the child node of Gather.
>
> This patch allows extensions to get control on the master backend's
> context when all the worker node gets finished but prior to release
> of the DSM segment. If FDW/CSP has its special statistics on the
> segment, it can move to the private memory area for EXPLAIN output
> or something other purpose.
>
> One design consideration is whether the hook shall be called from
> ExecParallelRetrieveInstrumentation() or ExecParallelFinish().
> The former is a function to retrieve the standard Instrumentation
> information, thus, it is valid only if EXPLAIN ANALYZE.
> On the other hands, if we put entrypoint at ExecParallelFinish(),
> extension can get control regardless of EXPLAIN ANALYZE, however,
> it also needs an extra planstate_tree_walker().
If the use case for this is to gather instrumentation, I'd suggest calling the
hook in RetrieveInstrumentation, and calling it appropriately. It would
make the intended use far clearer than it is now.
And if it saves some work, all the better.
Until there's a use case for a non-instrumentation hook in that place,
I wouldn't add it. This level of generality sounds like a solution waiting
for a problem to solve.