Horton

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

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Maciek Sakrejda <m.sakrejda@gmail.com>, Lukas Fittl <lukas@fittl.com>, 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-26T18:58:08Z
Lists: pgsql-hackers 
Hi,

On 2022-10-24 14:38:52 -0400, Melanie Plageman wrote:
> > - "repossession" is a very unintuitive name for me. If we want something like
> >   it, can't we just name it reuse_failed or such?
>
> Repossession could be called eviction_failed or reuse_failed.
> Do we think we will ever want to use it to count buffers we released
> in other IOContexts (thus making the name eviction_failed better than
> reuse_failed)?

I've a somewhat radical proposal: Let's just not count any of this in the
initial version. I think we want something, but clearly it's one of the harder
aspects of this patch. Let's get the rest in, and then work on this is in
isolation.


> Speaking of IOCONTEXT_LOCAL, I was wondering if it is confusing to call
> it IOCONTEXT_LOCAL since it refers to IO done for temporary tables. What
> if, in the future, we want to track other IO done using data in local
> memory?

Fair point. However, I think 'tmp' or 'temp' would be worse, because there's
other sources of temporary files that would be worth counting, consider
e.g. tuplestore temporary files. 'temptable' isn't good because it's not just
tables. 'temprel'? On balance I think local is better, but not sure.


> Also, what if we want to track other IO done using data from shared memory
> that is not in shared buffers? Would IOCONTEXT_SB and IOCONTEXT_TEMP be
> better? Should IOContext literally describe the context of the IO being done
> and there be a separate column which indicates the source of the data for
> the IO?  Like wal_buffer, local_buffer, shared_buffer? Then if it is not
> block-oriented, it could be shared_mem, local_mem, or bypass?

Hm. I don't think we'd need _buffer for WAL or such, because there's nothing
else.


> If we had another dimension to the matrix "data_src" which, with
> block-oriented IO is equivalent to "buffer type", this could help with
> some of the clarity problems.
>
> We could remove the "reused" column and that becomes:
>
> IOCONTEXT |     DATA_SRC    | IOOP
> ----------------------------------------
> strategy  | strategy_buffer | EVICT

> Having data_src and iocontext simplifies the meaning of all io
> operations involving a strategy. Some operations are done on shared
> buffers and some on existing strategy buffers and this would be more
> clear without the addition of special columns for strategies.

-1, I think this just blows up the complexity further, without providing much
benefit. But:

Perhaps a somewhat similar idea could be used to address the concerns in the
preceding paragraphs. How about the following set of columns:

backend_type:
object: relation, temp_relation[, WAL, tempfiles, ...]
iocontext: buffer_pool, bulkread, bulkwrite, vacuum[, bypass]
read:
written:
extended:
bytes_conversion:
evicted:
reused:
files_synced:
stats_reset:

Greetings,

Andres Freund

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.