public inbox for [email protected]help / color / mirror / Atom feed
BUFFERS enabled by default in EXPLAIN (ANALYZE) 7+ messages / 6 participants [nested] [flat]
* BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-11-12 22:58 Nikolay Samokhvalov <[email protected]> 0 siblings, 3 replies; 7+ messages in thread From: Nikolay Samokhvalov @ 2021-11-12 22:58 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> Re-reading the thread [1] (cannot answer there – don't have those emails in my box anymore), I see that there was strong support for enabling BUFFERS in EXPLAIN ANALYZE by default. And there were patches. Commitfest entry [2] was marked Rejected because there were questions to the implementation based on GUCs. Attached is a simple patch enabling BUFFERS by default, without involving GUCs. Why it is important? In many cases, people forget about the BUFFERS option in EXPLAIN ANALYZE and share execution plans without it – sending it via regular communication channels for work or publishing to visualization systems. Meanwhile, the option has a lower overhead compared to TIMING (enabled by default for EXPLAIN ANALYZE) and it is extremely useful for query optimization. This patch doesn't enable BUFFERS for EXPLAIN executed without ANALYZE. Open questions: 1. Should BUFFERS be enabled for regular (w/o ANALYZE) EXPLAIN? Now it may make sense because of the buffer numbers the planner uses. This patch doesn't do that, but it definitely may make sense because it can help people understand why planning time is so big, in some cases. 2. How to adjust documentation? Should EXPLAIN ANALYZE examples be adjusted to use BUFFERS OFF (easier change) or show some example buffer numbers – like it is done for timing and cost numbers? I tend to think that the latter makes more sense. 3. How to adjust regression tests? Of course, now many tests fail. Same question as for documentation. Excluding buffer, numbers would be an easier fix, of course – but at least some tests should check the behavior of BUFFERS (such as both default options – with and without ANALYZE). On any given platform, the buffer numbers are pretty stable, so we could rely on it, but I'm not sure about all possible options being tested and would appreciate advice here (of course, if the patch makes it thru the discussion in general). ## Links [1] https://www.postgresql.org/message-id/flat/b3197ba8-225f-f53c-326d-5b1756c77c3e%40postgresfriends.or... [2] https://commitfest.postgresql.org/28/2567/ Attachments: [application/octet-stream] 001-buffers-in-explain-analyze-enabled-by-default.patch (3.0K, ../../CANNMO++=LrJ4upoeydZhbmpd_ZgZjrTLueKSrivn6xmb=yFwQw@mail.gmail.com/3-001-buffers-in-explain-analyze-enabled-by-default.patch) download | inline diff: From d70b147d71ca15beb51187b75645587ed7de612f Mon Sep 17 00:00:00 2001 From: NikolayS <[email protected]> Date: Fri, 12 Nov 2021 21:55:47 +0000 Subject: [PATCH] Enable BUFFERS by default in EXPLAIN ANALYZE In many cases, people forget about the BUFFERS option in EXPLAIN ANALYZE and share execution plans without it. Meanwhile, the option has a lower overhead compared to TIMING (enabled by default for EXPLAIN ANALYZE) and it is extremely useful for query optimization. This patch doesn't enable BUFFERS for EXPLAIN executed without ANALYZE. See also: https://www.postgresql.org/message-id/flat/b3197ba8-225f-f53c-326d-5b1756c77c3e%40postgresfriends.org --- doc/src/sgml/ref/explain.sgml | 8 +++++--- src/backend/commands/explain.c | 10 ++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml index 4d758fb237..57a16ac32f 100644 --- a/doc/src/sgml/ref/explain.sgml +++ b/doc/src/sgml/ref/explain.sgml @@ -188,9 +188,11 @@ ROLLBACK; previously-dirtied blocks evicted from cache by this backend during query processing. The number of blocks shown for an - upper-level node includes those used by all its child nodes. In text - format, only non-zero values are printed. It defaults to - <literal>FALSE</literal>. + upper-level node includes those used by all its child nodes. In text + format, only non-zero values are printed. It defaults to + <literal>TRUE</literal> for <literal>EXPLAIN ANALYZE</literal> and + to <literal>FALSE</literal> for <literal>EXPLAIN</literal> executed + without the <literal>ANALYZE</literal> option. </para> </listitem> </varlistentry> diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 10644dfac4..85491defc8 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -171,6 +171,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, List *rewritten; ListCell *lc; bool timing_set = false; + bool buffers_set = false; bool summary_set = false; /* Parse options list. */ @@ -185,7 +186,10 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, else if (strcmp(opt->defname, "costs") == 0) es->costs = defGetBoolean(opt); else if (strcmp(opt->defname, "buffers") == 0) + { + buffers_set = true; es->buffers = defGetBoolean(opt); + } else if (strcmp(opt->defname, "wal") == 0) es->wal = defGetBoolean(opt); else if (strcmp(opt->defname, "settings") == 0) @@ -241,6 +245,12 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("EXPLAIN option TIMING requires ANALYZE"))); + /* if the buffers option was not set explicitly, set default value: + * - TRUE for EXPLAIN ANALYZE + * - FALSE for EXPLAIN without ANALYZE + */ + es->buffers = (buffers_set) ? es->buffers : es->analyze; + /* if the summary was not set explicitly, set default value */ es->summary = (summary_set) ? es->summary : es->analyze; -- GitLab ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-11-13 00:18 Tomas Vondra <[email protected]> parent: Nikolay Samokhvalov <[email protected]> 2 siblings, 1 reply; 7+ messages in thread From: Tomas Vondra @ 2021-11-13 00:18 UTC (permalink / raw) To: Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]> On 11/12/21 23:58, Nikolay Samokhvalov wrote: > Re-reading the thread [1] (cannot answer there – don't have those emails > in my box anymore), You can download the message as mbox and import it into your client (pretty much any client supports that, I think). regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-11-13 01:34 Vik Fearing <[email protected]> parent: Nikolay Samokhvalov <[email protected]> 2 siblings, 0 replies; 7+ messages in thread From: Vik Fearing @ 2021-11-13 01:34 UTC (permalink / raw) To: Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]> On 11/12/21 11:58 PM, Nikolay Samokhvalov wrote: > Re-reading the thread [1] (cannot answer there – don't have those emails in > my box anymore), I see that there was strong support for enabling BUFFERS > in EXPLAIN ANALYZE by default. And there were patches. Commitfest entry [2] > was marked Rejected because there were questions to the implementation > based on GUCs. > > Attached is a simple patch enabling BUFFERS by default, without involving > GUCs. I obviously agree with this (although I still wish we had gucs as we keep adding more and more options to EXPLAIN). The patch looks good to me, too. +1 -- Vik Fearing ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-11-13 04:17 Julien Rouhaud <[email protected]> parent: Tomas Vondra <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Julien Rouhaud @ 2021-11-13 04:17 UTC (permalink / raw) To: Tomas Vondra <[email protected]>; +Cc: Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]> On Sat, Nov 13, 2021 at 8:18 AM Tomas Vondra <[email protected]> wrote: > > On 11/12/21 23:58, Nikolay Samokhvalov wrote: > > Re-reading the thread [1] (cannot answer there – don't have those emails > > in my box anymore), > > You can download the message as mbox and import it into your client > (pretty much any client supports that, I think). There is also a "resend mail" link available for each mail in the archive which should be even more straightforward. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-11-15 19:09 Justin Pryzby <[email protected]> parent: Nikolay Samokhvalov <[email protected]> 2 siblings, 2 replies; 7+ messages in thread From: Justin Pryzby @ 2021-11-15 19:09 UTC (permalink / raw) To: Nikolay Samokhvalov <[email protected]>; +Cc: [email protected] On Fri, Nov 12, 2021 at 02:58:07PM -0800, Nikolay Samokhvalov wrote: > Re-reading the thread [1] (cannot answer there – don't have those emails in > my box anymore), I see that there was strong support for enabling BUFFERS > in EXPLAIN ANALYZE by default. And there were patches. Commitfest entry [2] > was marked Rejected because there were questions to the implementation > based on GUCs. I think the only reason this isn't already the default is because of (3): > 3. How to adjust regression tests? Of course, now many tests fail. Same > question as for documentation. Excluding buffer, numbers would be an easier > fix, of course – but at least some tests should > check the behavior of BUFFERS (such as both default options – with and > without ANALYZE). Some time ago, I had a few relevant patches: 1) add explain(REGRESS) which is shorthand for (BUFFERS OFF, TIMING OFF, COSTS OFF, SUMMARY OFF) 2) add explain(MACHINE) which elides machine-specific output from explain; for example, Heap Fetches, sort spaceUsed, hash nbuckets, and tidbitmap stuff. https://www.postgresql.org/message-id/flat/[email protected] > 1. Should BUFFERS be enabled for regular (w/o ANALYZE) EXPLAIN? Now it may > make sense because of the buffer numbers the planner uses. This patch > doesn't do that, but it definitely may make sense because it can help > people understand why planning time is so big, in some cases. I think it *should* be enabled for planning, since that makes the default easier to understand and document, and it makes a user's use of "explain" easier. However, that makes this patch series more complicated: explain(ANALYZE) is rare in the regression tests, but this would affect the output of bare "explain", which affects almost every test. I think the answer to that is to add a GUC (as Tom suggested in an old thread) like explain_regress=on, which causes explain to omit these details. This would be instead of my explain(REGRESS), and would change the defaults of various params in a central place, to avoid the need to update many regression tests. In the future, this might also handle the stuff that my "explain(MACHINE)" attempted to handle. I've rebased my patches like this. I think the explain(REGRESS) should be re-written as a GUC which is set by regress.c. I'm interested to hear from a reviewer if this is worth pursing like this. -- Justin ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-11-24 18:35 Michael Christofides <[email protected]> parent: Justin Pryzby <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Michael Christofides @ 2021-11-24 18:35 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Nikolay Samokhvalov <[email protected]>; [email protected] I think it *should* be enabled for planning, since that makes the default > easier to understand and document, and it makes a user's use of "explain" > easier. I’d be keen to see BUFFERS off by default with EXPLAIN, and on by default with EXPLAIN ANALYZE. The SUMMARY flag was implemented that way, which I think has been easy enough for folks to understand and document. In fact, I think the only BUFFERS information goes in the “summary” section for EXPLAIN (BUFFERS), so maybe it makes perfect sense? If it would be great if that made implementation easier, too. In any case, thank you all, I’m so glad that this is being discussed again. It’d be so good to start seeing buffers in more plans. — Michael ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUFFERS enabled by default in EXPLAIN (ANALYZE) @ 2021-12-02 00:58 Justin Pryzby <[email protected]> parent: Justin Pryzby <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Justin Pryzby @ 2021-12-02 00:58 UTC (permalink / raw) To: Nikolay Samokhvalov <[email protected]>; +Cc: [email protected] On Mon, Nov 15, 2021 at 01:09:54PM -0600, Justin Pryzby wrote: > Some time ago, I had a few relevant patches: > 1) add explain(REGRESS) which is shorthand for (BUFFERS OFF, TIMING OFF, COSTS OFF, SUMMARY OFF) > 2) add explain(MACHINE) which elides machine-specific output from explain; > for example, Heap Fetches, sort spaceUsed, hash nbuckets, and tidbitmap stuff. > > https://www.postgresql.org/message-id/flat/[email protected] The attached patch series now looks like this (some minor patches are not included in this list): 1. add GUC explain_regress, which disables TIMING, SUMMARY, COSTS; 2. enable explain(BUFFERS) by default (but disabled by explain_regress); 3. Add explain(MACHINE) - which is disabled by explain_regress. This elides various machine-specific output like Memory and Disk use. Maybe it should be called something else like "QUIET" or "VERBOSE_MINUS_1" or ?? The regression tests now automatically run with explain_regress=on, which is shorthand for TIMING OFF, SUMMARY OFF, COSTS OFF, and then BUFFERS OFF. There's a further option called explain(MACHINE) which can be disabled to hide portions of the output that are unstable, like Memory/Disk/Batches/ Heap Fetches/Heap Blocks. This allows "explain analyze" to be used more easily in regression tests, and simplifies some existing tests that currently use plpgsql functions to filter the output. But it doesn't handle all the variations from parallel workers. (3) is optional, but simplifies some regression tests. The patch series could be rephrased with (3) first. Unfortunately, "COSTS OFF" breaks postgres_fdw remote_estimate. If specifying "COSTS ON" in postgres_fdw.c is considered to be a poor fix , then I suppose this patch series could do as suggested and enable buffers by default only when ANALYZE is specified. Then postgres_fdw is not affected, and the explain_regress GUC is optional: instead, we'd need to specify BUFFERS OFF in ~100 regression tests which use EXPLAIN ANALYZE. (3) still seems useful on its own. ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2021-12-02 00:58 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-11-12 22:58 BUFFERS enabled by default in EXPLAIN (ANALYZE) Nikolay Samokhvalov <[email protected]> 2021-11-13 00:18 ` Tomas Vondra <[email protected]> 2021-11-13 04:17 ` Julien Rouhaud <[email protected]> 2021-11-13 01:34 ` Vik Fearing <[email protected]> 2021-11-15 19:09 ` Justin Pryzby <[email protected]> 2021-11-24 18:35 ` Michael Christofides <[email protected]> 2021-12-02 00:58 ` Justin Pryzby <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox