On Thu, Apr 8, 2021 at 12:18 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Thu, Apr 08, 2021 at 12:08:02PM +0530, Amit Kapila wrote:
> > On Thu, Apr 8, 2021 at 11:40 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > >
> > > That was indeed the problem. I think the best is to entirely ignore parallel
> > > workers in pg_stat_statements,
> > >
> >
> > I haven't studied this patch but I have a question here. We normally
> > do accumulate the stats from parallel workers for the purpose of
> > Explain Analyze, so won't ignoring them in pg_stat_statements a bit
> > inconsistent but if that is the case even before this patch then that
> > might be fine?
>
> I think that this patch shouldn't change pg_stat_statements previous behavior,
> and that the previous behavior was correct.
>
> It doesn't change the way we get instrumentation data from parallel workers,
> just avoid to have parallel workers call pgss_store and report multiple time
> the same activity. The only difference is that now pg_stat_statements won't
> setup instrumentation flags, but those should be inherited in parallel worker
> from the main process right?
>
Yeah, we do pass instrumentation flags from leader to worker but not
the one allocated via pg_stat_staments (pgss_ExecutorStart {...
queryDesc->totaltime ...}. However, I guess prior to this patch those
were anyway not allocated separately because queryid was not passed to
workers. So, I think you are right it shouldn't change the previous
behavior.
I am not completely sure but it seems we were not accumulating the
instrumentation info by workers required by pg_stat_statements neither
before this patch nor after but I might be missing something.
--
With Regards,
Amit Kapila.