On Wed, Dec 6, 2017 at 12:02 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Tue, Dec 5, 2017 at 1:29 PM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>> Or would it be an option to change the time
>> ExecXXXRetrieveInstrumentation() is called so it is run only once?
>
> To me, that doesn't sound like a bad option. I think if do so, then
> we don't even need to reinitialize the shared sort stats. I think
> something, like attached, should work if we want to go this route. We
> can add regression test if this is what we think is a good idea.
> Having said that, one problem I see doing thing this way is that in
> general, we will display the accumulated stats of each worker, but for
> sort or some other special nodes (like hash), we will show the
> information of only last loop. I am not sure if that is a matter of
> concern, but if we want to do this way, then probably we should
> explain this in documentation as well.
The hash version of this code is now committed as 5bcf389e. Here is a
patch for discussion that adds some extra tests to join.sql to
exercise rescans of a hash join under a Gather node. It fails on
head, because it loses track of the instrumentation pointer after the
first loop as you described (since the Hash coding is the same is the
Sort coding), so it finishes up with no instrumentation data. If you
move ExecParallelRetrieveInstrumentation() to ExecParallelCleanup() as
you showed in your patch, then it passes. The way I'm asserting that
instrumentation data is making its way back to the leader is by
turning off leader participation and then checking if it knows how
many batches there were.
--
Thomas Munro
http://www.enterprisedb.com
