On Thu, Nov 7, 2019 at 11:37 PM Rafia Sabih <rafia.pghack...@gmail.com> wrote: > I was reviewing this patch and here are a few comments,
Hi Rafia,
Thanks!
> +static void
> +ExplainNodePerProcess(ExplainState *es, bool *opened_group,
> + int worker_number, Instrumentation *instrument)
> +{
>
> A small description about this routine would be helpful and will give the
> file a consistent look.
Done for both new functions. I also improved the commit message for
0001 a bit to explain the change better.
> Also, I noticed that the worker details are displayed for sort node even
> without verbose, but for scans it is only with verbose. Am I missing
> something or there is something behind? However, I am not sure if this is the
> introduced by this patch-set.
Yeah, it's a pre-existing thing, but I agree it's an interesting
difference. We currently don't have a way to show a 'combined'
version of a parallel (oblivious) sort: we always show the per-process
version, and all this patch changes is how we label the leader's
stats. I suppose someone could argue that in non-VERBOSE mode we
should show the total memory usage (sum from all processes). I suppose
it's possible they use different sort types (one worker runs out of
work_mem and another doesn't), and I'm not sure how how you'd
aggregate that.
0001-Show-parallel-leader-stats-in-EXPLAIN-output-v3.patch
Description: Binary data
0002-Improve-EXPLAIN-of-Sort-in-parallel-queries-v3.patch
Description: Binary data
