| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Report buffer access strategies in EXPLAIN |
| Date: | 2026-10-29 08:23:13 |
| Message-ID: | auMCcWJ/K9CE+VpP@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Wed, Oct 28, 2026 at 04:21:49PM -0500, Sami Imseih wrote:
> There is a core case here too.
Oh right, thanks for looking at it!
> I still do not think we need to show all the strategies used here, at least not
> for the first version of this patch. What the user will care about most is
> which ring strategy was used, so if anything other than normal was used, that
> should take precedence. This should be documented.
I had more thoughts about it and I think that both strategies are important, as
both affect buffer replacement and therefore may impact the workload.
What about retaining all strategies observed across rescans? The instrumentation
could use a bitmask and EXPLAIN would report them as a list, for example:
Buffer Access Strategies: normal, bulkread
That would preserve all relevant information without giving one strategy precedence.
Would that make sense?
> I also have a few additional comments by patch:
>
> v2-0002:
>
> The glossary still reads as if all Buffer Access Strategies are ring-based, but
> normal is listed below and explicitly does not use a strategy ring. I think this
> wording would avoid that:
>
> ```
> - A Buffer Access Strategy sets up references to a limited number of
> - <glossterm linkend="glossary-shared-memory">shared buffers</glossterm> and
> - reuses them circularly. When the operation requires a new page, a victim
> - buffer is chosen from the buffers in the strategy ring, which may require
> + Except for <literal>normal</literal>, Buffer Access Strategies set up
> + references to a limited number of
> + <glossterm linkend="glossary-shared-memory">shared buffers</glossterm>
> + and reuse them circularly. When the operation requires a new page, a
> + victim buffer is chosen from the buffers in the strategy ring, which may
> + require
> ```
Agree, will do.
> v2-0003:
>
> In the pg_stat_io docs, I think the bulkwrite example should say COPY FROM
> rather than COPY:
Agreed.
> v2-0007:
>
> I think the EXPLAIN docs should clarify that this field does not report
> bulkwrite.
I think that was implied by "plan nodes report the strategies used by the
table or index scan" but better to remove ambiguity as you suggest.
> v2-0008:
>
> I think InstrRecordIndexScanStrategy(..., BAS_BULKREAD) should move to
> blgetbitmap(), just before GetAccessStrategy(BAS_BULKREAD), after the NULL-key
> early return
Good catch, will do that.
> Maybe as a follow-up, but showing the ring size too seems useful also.
Yeah, I agree that reporting the ring size could be useful. As mentioned in the
first email, I initially planned to report it, together with ring activity
counters, but after discussing it with Andres at PGConf.EU we agreed to keep the
first version simple. So I’d prefer to leave that for a follow-up.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-10-29 08:26:24 | Re: [PATCH] pgstat: Add helper to drop entries from a stats kind's hash |
| Previous Message | Michael Paquier | 2026-10-29 08:14:43 | Re: Return pg_control from pg_backup_stop(). |