From: | "Imseih (AWS), Sami" <simseih(at)amazon(dot)com> |
---|---|
To: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PROPOSAL] Detecting plan changes with plan_id in pg_stat_activity |
Date: | 2022-06-16 21:32:26 |
Message-ID: | 70C9C1F5-BE5C-4008-98F3-88DE09900D0E@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> AFAICS you're proposing to add an identifier for a specific plan, but no way to
> know what that plan was? How are users supposed to use the information if they
> know something changed but don't know what changed exactly?
I see this as a start to do more useful things with plans.
The patch right out the gate exposes the plan_id in EXPLAIN output
and auto_explain.
This will also be useful for extensions that will provide the plan text.
It is also conceivable that pg_stat_statements can provide an option
To store the plan text?
> - In the POC, the compute_query_id GUC determines if a
> plan_id is to be computed. Should this be a separate GUC?
> Probably, as computing it will likely be quite expensive. Some benchmark on
> various workloads would be needed here.
Yes, more benchmarks will be needed here with more complex plans. I have
Only benchmarked with pgbench at this point.
However, separating this into Its own GUC is what I am leaning towards as well
and will update the patch.
> I only had a quick look at the patch, but I see that you have some code to
> avoid storing the query text multiple times with different planid. How does it
> work exactly, and does it ensure that the query text is only removed once the
> last entry that uses it is removed? It seems that you identify a specific
> query text by queryid, but that seems wrong as collision can (easily?) happen
> in different databases. The real identifier of a query text should be (dbid,
> queryid).
The idea is to lookup the offsets and length of the text in the external file by querid
only. Therefore we can store similar query text for multiple pgss_hash entries
only once.
When a new entry in pgss is not found, the new qtext_hash is consulted to
see if it has information about the offsets/length of the queryid. If found in
qtext_hash, the new pgss_hash entry is created with the offsets found.
If not found in qtext_hash, the query text will be (normalized) and stored in
the external file. Then, a new entry will be created in qtext_hash and
an entry in pgss_hash.
Of course we need to also handle the gc_qtext cleanups, entry_reset, startup
and shutdown scenarios. The patch does this, but I will go back and do more
testing.
> Note that this problem already exists, as the query texts are now stored per
> (userid, dbid, queryid, istoplevel). Maybe this part could be split in a
> different commit as it could already be useful without a planid.
Good point. I will separate this patch.
Regards,
Sami Imseih
Amazon Web Services
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2022-06-16 22:30:14 | Nothing is using StrategyNotifyBgWriter() anymore |
Previous Message | Bruce Momjian | 2022-06-16 20:29:52 | Re: Typo in ro.po file? |