Horton

Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

Maciek Sakrejda <m.sakrejda@gmail.com>

View thread View on postgr.es
From: Maciek Sakrejda <m.sakrejda@gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Lukas Fittl <lukas@fittl.com>, Andres Freund <andres@anarazel.de>, Justin Pryzby <pryzby@telsasoft.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Magnus Hagander <magnus@hagander.net>, Pg Hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>
Date: 2022-10-23T22:35:38Z
Lists: pgsql-hackers 
On Wed, Oct 19, 2022 at 12:27 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> v34 is attached.
> I think the column names need discussion. Also, the docs need more work
> (I added a lot of new content there). I could use feedback on the column
> names and definitions and review/rephrasing ideas for the docs
> additions.

Nice! I think the expanded docs are great, and make this information
much easier to interpret.

>+       <varname>io_context</varname> <literal>bulkread</literal>, existing
>+       dirty buffers in the ring requirng flush are

"requiring"

>+       shared buffers were acquired from the freelist and added to the
>+       fixed-size strategy ring buffer. Shared buffers are added to the
>+       strategy ring lazily. If the current buffer in the ring is pinned or in

This is the first mention of the term "strategy" in these docs. It's
not totally opaque, since there's some context, but maybe we should
either try to avoid that term or define it more explicitly?

>+       <varname>io_context</varname>s. This is equivalent to
>+       <varname>evicted</varname> for shared buffers in
>+       <varname>io_context</varname> <literal>shared</literal>, as the contents
>+       of the buffer are <quote>evicted</quote> but refers to the case when the

I don't quite follow this: does this mean that I should expect
'reused' and 'evicted' to be equal in the 'shared' context, because
they represent the same thing? Or will 'reused' just be null because
it's not distinct from 'evicted'? It looks like it's null right now,
but I find the wording here confusing.

>+      future with a new shared buffer. A high number of
>+      <literal>bulkread</literal> rejections can indicate a need for more
>+      frequent vacuuming or more aggressive autovacuum settings, as buffers are
>+      dirtied during a bulkread operation when updating the hint bit or when
>+      performing on-access pruning.

This is great. Just wanted to re-iterate that notes like this are
really helpful to understanding this view.

> I've implemented a change using the same function pg_settings uses to
> turn the build-time parameter BLCKSZ into 8kB (get_config_unit_name())
> using the flag GUC_UNIT_BLOCKS. I am unsure if this is better or worse
> than "block_size". I am feeling very conflicted about this column.

Yeah, I guess it feels less natural here than in pg_settings, but it
still kind of feels like one way of doing this is better than two...

Commits

  1. Stabilize pg_stat_io writes test

  2. Fix flakey pg_stat_io test

  3. Suppress more compiler warnings in new pgstats code.

  4. Suppress compiler warnings in new pgstats code.

  5. Add tests for pg_stat_io

  6. Create regress_tblspc in test_setup

  7. Add pg_stat_io view, providing more detailed IO statistics

  8. pgstat: Track more detailed relation IO statistics

  9. pgstat: Infrastructure for more detailed IO statistics

  10. doc: Fix some issues in logical replication section

  11. Manual cleanup and pgindent of pgstat and bufmgr related code

  12. Have the planner consider Incremental Sort for DISTINCT

  13. Use actual backend IDs in pg_stat_get_backend_idset() and friends.

  14. Remove redundant call to pgstat_report_wal()

  15. Add BackendType for standalone backends

  16. Initialize backend status reporting during bootstrap.