On Thu, Nov 7, 2019 at 11:37 PM Rafia Sabih <rafia.pghackers@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
withverbose. Am I missing something or there is something behind? However, I am not sure if this is the introduced by
thispatch-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.