Re: [PATCH] Add extra statistics to explain for Nested Loop

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Ekaterina Sokolova <e(dot)sokolova(at)postgrespro(dot)ru>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Lukas Fittl <lukas(at)fittl(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Add extra statistics to explain for Nested Loop
Date: 2022-04-02 12:38:26
Message-ID: 20220402123826.GE28503@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

This message lost track of the email headers so CFBOT isn't processing the new
patches. Which I'm attempting to remedy now.
https://www.postgresql.org/message-id/flat/ae576cac3f451d318374f2a2e494aab1(at)postgrespro(dot)ru

On Fri, Apr 01, 2022 at 11:46:47PM +0300, Ekaterina Sokolova wrote:
> Hi, hackers. Thank you for your attention to this topic.
>
> Julien Rouhaud wrote:
> > +static void show_loop_info(Instrumentation *instrument, bool isworker,
> > + ExplainState *es);
> >
> > I think this should be done as a separate refactoring commit.
> Sure. I divided the patch. Now Justin's refactor commit is separated. Also I
> actualized it a bit.
>
> > Most of the comments I have are easy to fix. But I think that the real
> > problem
> > is the significant overhead shown by Ekaterina that for now would apply
> > even if
> > you don't consume the new stats, for instance if you have
> > pg_stat_statements.
> > And I'm still not sure of what is the best way to avoid that.
> I took your advice about InstrumentOption. Now INSTRUMENT_EXTRA exists.
> So currently it's no overheads during basic load. Operations using
> INSTRUMENT_ALL contain overheads (because of INSTRUMENT_EXTRA is a part of
> INSTRUMENT_ALL), but they are much less significant than before. I apply new
> overhead statistics collected by pgbench with auto _explain enabled.
>
> > Why do you need to initialize min_t and min_tuples but not max_t and
> > max_tuples while both will initially be 0 and possibly updated
> > afterwards?
> We need this initialization for min values so comment about it located above
> the block of code with initialization.
>
> I am convinced that the latest changes have affected the patch in a positive
> way. I'll be pleased to hear your thoughts on this.

Attachment Content-Type Size
0001-explain.c-refactor-ExplainNode.patch text/x-diff 4.9 KB
0002-Add-extra-statistics-to-explain-for-Nested-Loop.patch text/x-diff 19.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2022-04-02 13:26:02 Re: psql - add SHOW_ALL_RESULTS option
Previous Message Andrei Zubkov 2022-04-02 12:02:22 Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements