> 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
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
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
> (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
> 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.
> 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
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
> 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
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
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
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
> 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 p
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
Sending rebased version to fix cfbot tests.
v3-0001-Proposal-for-progressive-explains.patch
Description: Binary data
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
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
Hello hackers,
Last Saturday I submitted a patch to the pgsql-hackers list with the title
"Proposal: In-flight explain logging" with a patch proposing a feature very
similar to the one being worked on in this thread. I should have done a
better
search in the commitfest before implementing somethin
Hello Maciek,
Thanks for pointing that out. They are indeed super similar. Before I wrote
the patch I searched for
"explain" related ones. I guess I should have performed a better search.
Comparing the patches, there is one main difference: the existing patch
prints only the plan without
any inst
Hello hackers,
# MOTIVATION
My recent experiences with problematic queries in customers motivated
me to write this patch proposing a new feature to enhance visibility
on what active queries are doing.
PostgreSQL already offers 2 very powerful tools for query troubleshooting:
- EXPLAIN: gives us
Hello all,
While investigating a problem in a PG14 instance I noticed that autovacuum
workers
stop processing other databases when a database has a temporary table with
age
older than `autovacuum_freeze_max_age`. To test that I added a custom
logline showing
which database the about to spawned aut
26 matches
Mail list logo
