> True, although thinking about it more, they're not being sent to the
> same place. auto_explain goes to the log, and this goes to a view.
> What about something like this:
> progressive_explain = on | off
> progressive_explain_inteval = 5s
> If progressive_explain is off, then we don't populate t
> I haven't looked into the code yet, but when I ran below commands during
> make installcheck, there was an error and an assertion failure
Thanks for the report. I actually made a nasty mistake in the last
patch after code refactoring, which is to not properly check that
a QueryDesc is already be
On Fri, Mar 28, 2025 at 12:09 PM Rafael Thofehrn Castro
wrote:
> I am definitely not the authority here to talk about the best way forward.
> If there is an agreement in turning this into an extension, it can be a new
> feature in auto_explain.
I'm not against adding some more hooks to explain.c,
On Fri, Mar 7, 2025 at 6:43 AM Rafael Thofehrn Castro
wrote:
The wrapper code was implemented by torikoshia
(torikoshia(at)oss(dot)nttdata(dot)com),
so adding the credits here.
On Thu, Mar 20, 2025 at 5:35 AM Robert Haas
wrote:
Without having the prior discussion near to hand, I *think* th
My bad, I mistakenly did the tests without assertion enabled in the last 2
days,
so couldn't catch that Assertion failure. Was able to reproduce it, thanks.
I guess when the code was designed we were not expecting to be doing
explains
in parallel workers.
One comment is that this has nothing to d
On Tue, Apr 1, 2025 at 9:38 AM torikoshia wrote:
> Could you please check if you can reproduce this?
I can, and I now see that this patch has a pretty big design problem.
The assertion failure occurs when a background worker tries to call
ruleutils.c's get_parameter(), which tries to find the exp
On 2025-04-01 15:23, Rafael Thofehrn Castro wrote:
Hello again,
ERROR: could not attach to dynamic shared area
In addition to that refactoring issue, the current patch had a race
condition in pg_stat_progress_explain to access the DSA of a process
running a query that gets aborted.
While di
Hello again,
> ERROR: could not attach to dynamic shared area
In addition to that refactoring issue, the current patch had a race
condition in pg_stat_progress_explain to access the DSA of a process
running a query that gets aborted.
While discussing with Robert we agreed that it would be wiser
Thanks for this valuable testing. I think this is actually a really
good idea for how to test something like this, because the regression
tests contain lots of different queries that do lots of weird things.
On Sun, Mar 30, 2025 at 8:23 PM torikoshia wrote:
> I haven't looked into the code yet, b
On 3/31/25 02:23, torikoshia wrote:
On Fri, Mar 7, 2025 at 6:43 AM Rafael Thofehrn Castro
Implemented this version. New patch has the following characteristics:
I haven't looked into the code yet, but when I ran below commands during
make installcheck, there was an error and an assertion fai
> (2) Merge progressive_explain = explain and progressive_explain =
> analyze into a single value, progressive_explain = on. To tell which
> is intended, just check progresive_explain_interval. If it's zero,
> then there's no repeat, so we mean what you currently call
> progressive_explain = explai
On Fri, Mar 28, 2025 at 4:12 PM Rafael Thofehrn Castro
wrote:
> > That is correct. Interval currently is only used when instrumentation
> > is enabled through progressive_explain = analyze.
>
> I guess there would still be a point in printing the plan on a regular
> interval
> when instrumentatio
> That is correct. Interval currently is only used when instrumentation
> is enabled through progressive_explain = analyze.
I guess there would still be a point in printing the plan on a regular
interval
when instrumentation is disabled. In that case the only thing we would see
changing in the pla
> I still have trouble understanding what that means. Is the interval
> irrelevant except when progressive_explain = analyze?
That is correct. Interval currently is only used when instrumentation
is enabled through progressive_explain = analyze.
On Fri, Mar 28, 2025 at 3:59 PM Rafael Thofehrn Castro
wrote:
> > True, although thinking about it more, they're not being sent to the
> > same place. auto_explain goes to the log, and this goes to a view.
> > What about something like this:
> > progressive_explain = on | off
> > progressive_expla
On Fri, Mar 28, 2025 at 12:09 PM Rafael Thofehrn Castro
wrote:
> As you said, visibility on the non instrumented query plan is already a
> feature
> of auto_explain.
True, although thinking about it more, they're not being sent to the
same place. auto_explain goes to the log, and this goes to a
> But having said that, I'm not quite sure I understand why you're
> proposing (A) and (B1) as separate alternatives. Changing
> progressive_explain to be a Boolean doesn't seem like it solves the
> problem of needing to wrap ExecProcNode from a signal handler. The
> only thing that seems to solve
On 3/26/25 22:57, Rafael Thofehrn Castro wrote:
So implementation was done based on transaction nested level. Cleanup is
only
performed when AbortSubTransaction() is called in the same transaction
nested
level as the one where the query is running. This covers both PL/pgSQL
exception
blocks an
On Thu, Mar 27, 2025 at 9:38 PM Rafael Thofehrn Castro
wrote:
> > Calling ProgressiveExplainTrigger() directly from
> > ProgressiveExplainStartupTimeoutHandler() seems extremely scary --
>
> Agreed. In that other similar patch to log query plans after a signal is sent
> from another session there
Thanks Robert. Very thorough analysis there.
Things I don't comment on will be fixed without further discussion.
> There is a comment in ExplainOnePlan() that says "Handle progressive
> explain cleanup manually if enabled as it is not called as part of
> ExecutorFinish," but standard_ExecutorFini
On Wed, Mar 26, 2025 at 5:58 PM Rafael Thofehrn Castro
wrote:
> So implementation was done based on transaction nested level. Cleanup is only
> performed when AbortSubTransaction() is called in the same transaction nested
> level as the one where the query is running. This covers both PL/pgSQL
>
> Suppose:
> BEGIN;
> SELECT 1;
> SAVEPOINT bob;
> progressively explain something that aborts
> I think in this case we will call AbortSubTransaction(), not
AbortTransaction().
Indeed. We need special treatment for subtransactions. There are 2
scenarios where
AbortSubTransaction() will be called
On Tue, Mar 25, 2025 at 8:52 PM Rafael Thofehrn Castro
wrote:
> This first version of the progressive explain feature was designed to only
> keep
> track of initial query called by the backend, ignoring all subquery calls. So
> I believe we don't need to worry about having to add custom logic in
Sending a new version that includes the new explain_progressive.c file in
meson.build.
Rafael.
v12-0001-Proposal-for-progressive-explains.patch
Description: Binary data
Hello Robert,
Fixed most of the recommendations. Going over one at a time.
> The documentation for the progressive_explain = { off | explain |
> analyze } option seems like it should go into more detail about how
> the "explain" and "analyze" values are different. I'm not 100% sure I
> know the a
On Wed, Mar 19, 2025 at 6:53 PM Rafael Thofehrn Castro
wrote:
> The strategy I used here is to use a MemoryContextCallback
> (ProgressiveExplainReleaseFunc), configured in the memory context
> where the query is being executed, being responsible for calling
> ProgressiveExplainCleanup() if the que
Hi Robert,
Thanks for sparing part of your precious time to look at the patch.
I acknowledge it is a very complex one. Since you're going to take
another look, providing some preliminary comments related to some
of the implementation concerns.
> I don't understand how this would be safe against i
On Wed, Mar 19, 2025 at 1:47 PM Rafael Thofehrn Castro
wrote:
> Sending a new version as rebase was required.
Reading this thread, it seems to me that there has been a good deal of
discussion about things like the name of the feature and what the UI
ought to be, but not much discussion of whether
Sending a new version as rebase was required.
Rafael.
v10-0001-Proposal-for-progressive-explains.patch
Description: Binary data
Thanks for the valuable inputs Euler. Adjusted most of your recommendations.
> I found a crash. It is simple to reproduce.
Indeed, I failed to test plain EXPLAIN after the addition of the new GUC
progressive_explain_min_duration. This is fixed.
> You call this feature "progressive explain". My f
On Fri, Mar 7, 2025, at 5:34 PM, Rafael Thofehrn Castro wrote:
> Did additional benchmarks and found issues with the patch that doesn't do
> execProcNode
> wrapping. There are sporadic crashes with **double free or corruption (top)**
>
> So making the patch that uses the wrapper the current
> Implementation of the new GUC progressive_explain_min_duration was done
> with
> timeouts. The timeout callback function is used to initialize the
> progressive
> explain.
>
> There is a catch to this implementation. In thread
> https://www.postgresql.org/message-id/flat/d68c3ae31672664876b22d2dc
Fixed a corner case where pg_stat_progress_explain is looking at its own
plan.
Previous message in this thread contains all relevant implementation
details of
the last patch.
On Tue, Feb 18, 2025 at 3:31 PM Rafael Thofehrn Castro
wrote:
> Hello all,
>
> Sending a new version of the patch that i
Hello all,
Sending a new version of the patch that includes important changes
addressing
feedback provided by Greg and Tomas. So, including the previous version (v5)
sent on Jan 29, these are the highlights of what has changed:
- Progressive plan printed on regular interval defined by
progressive
Hi,
Thanks for the valuable feedback Tomas. I am sending a new version of the
patch
that includes:
- Changed instrumented plan printing based on timeouts instead of sampling.
This works perfectly and benchmarks are promising. So new GUC
progressive_explain_sampe_rate is removed.
- Removed all par
On 12/30/24 2:18 AM, Rafael Thofehrn Castro wrote:
Hello community,
CONTEXT:
Back in October I presented the talk "Debugging active queries with
mid-flight instrumented explain plans" at PGConf EU 2024
(recording: https://www.youtube.com/watch?v=6ahTb-7C05c) presenting
an experimental feature t
Hi Rafael,
This sounds like a great feature, thanks for working on it and sharing
the patch. Let me share some comments / questions after a quick review.
On 12/30/24 02:18, Rafael Thofehrn Castro wrote:
> Hello community,
>
> CONTEXT:
>
> Back in October I presented the talk "Debugging active q
Sending rebased version to fix cfbot tests.
v3-0001-Proposal-for-progressive-explains.patch
Description: Binary data
hi.
all the newly added GUC
progressive_explain;
progressive_explain_verbose;
progressive_explain_settings;
progressive_explain_interval;
progressive_explain_output_size;
progressive_explain_format;
progressive_explain_sample_rate;
also need to add to postgresql.conf.sample?
in doc/src/sgml/mon
Thanks Greg, Sami and Jian for the feedback so far.
> Maybe track_explain instead? In the spirit of track_activity.
That was the original name, and all other GUCs were following the
track_activity_* logic. Changed to the name of the feature after
discussion with colleagues at EDB. This is definit
hi.
[48/208] Compiling C object
contrib/postgres_fdw/postgres_fdw.so.p/postgres_fdw.c.o
FAILED: contrib/postgres_fdw/postgres_fdw.so.p/postgres_fdw.c.o
/usr/local/gcc-14.1.0/bin/gcc-14.1.0
-Icontrib/postgres_fdw/postgres_fdw.so.p -Isrc/include
-I../../Desktop/pg_src/src5/postgres/src/include
-Isrc
> This proposal introduces a feature to print execution plans of active
> queries in an in-memory shared hash object so that other sessions can
> visualize them with a new view: pg_stat_progress_explain.
Thanks for this thread and for sharing the presentation
material. +1 for the idea of adding in
On Sun, Dec 29, 2024 at 8:19 PM Rafael Thofehrn Castro
wrote:
> Plans are only printed if the new GUC parameter progressive_explain is
> enabled.
>
Maybe track_explain instead? In the spirit of track_activity.
- progressive_explain_output_size: max output size of the plan printed in
> the in-me
Hello community,
CONTEXT:
Back in October I presented the talk "Debugging active queries with
mid-flight instrumented explain plans" at PGConf EU 2024
(recording: https://www.youtube.com/watch?v=6ahTb-7C05c) presenting
an experimental feature that enables visualization of in progress
EXPLAIN ANAL
44 matches
Mail list logo
