Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Andres Freund <andres@anarazel.de>
Hi, - we shouldn't do pgstat_count_io_op() while the buffer header lock is held, if possible. I wonder if we should add a "source" output argument to StrategyGetBuffer(). Then nearly all the counting can happen in BufferAlloc(). - "repossession" is a very unintuitive name for me. If we want something like it, can't we just name it reuse_failed or such? - Wonder if the column names should be reads, writes, extends, etc instead of the current naming pattern - Is it actually correct to count evictions in StrategyGetBuffer()? What if we then decide to not use that buffer in BufferAlloc()? Yes, that'll be counted via rejected, but that still leaves the eviction count to be "misleading"? On 2022-10-19 15:26:51 -0400, Melanie Plageman wrote: > I have made some major changes in this area to make the columns more > useful. I have renamed and split "clocksweeps". It is now "evicted" and > "freelist acquired". This makes it clear when a block must be evicted > from a shared buffer must be and may help to identify misconfiguration > of shared buffers. I'm not sure freelist acquired is really that useful? If we don't add it, we should however definitely not count buffers from the freelist as evictions. > There is some nuance here that I tried to make clear in the docs. > "freelist acquired" in a shared context is straightforward. > "freelist acquired" in a strategy context is counted when a shared > buffer is added to the strategy ring (not when it is reused). Not sure what the second half here means - why would a buffer that's not from the freelist ever be counted as being from the freelist? > "freelist_acquired" is confusing for local buffers but I wanted to > distinguish between reuse/eviction of local buffers and initial > allocation. "freelist_acquired" seemed more fitting because there is a > clocksweep to find a local buffer and if it hasn't been allocated yet it > is allocated in a place similar to where shared buffers acquire a buffer > from the freelist. If I didn't count it here, I would need to make a new > column only for local buffers called "allocated" or something like that. I think you're making this too granular. We need to have more detail than today. But we don't necessarily need to catch every nuance. > I chose not to call "evicted" "sb_evicted" > because then we would need a separate "local_evicted". I could instead > make "local_evicted", "sb_evicted", and rename "reused" to > "strat_evicted". If I did that we would end up with separate columns for > every IO Context describing behavior when a buffer is initially acquired > vs when it is reused. > > It would look something like this: > > shared buffers: > initial: freelist_acquired > reused: sb_evicted > > local buffers: > initial: allocated > reused: local_evicted > > strategy buffers: > initial: sb_evicted | freelist_acquired > reused: strat_evicted > replaced: sb_evicted | freelist_acquired > > This seems not too bad at first, but if you consider that later we will > add other kinds of IO -- eg WAL IO or temporary file IO, we won't be > able to use these existing columns and will need to add even more > columns describing the exact behavior in those cases. I think it's clearly not the right direction. > I have also added the columns "repossessed" and "rejected". "rejected" > is when a bulkread rejects a strategy buffer because it is dirty and > requires flush. Seeing a lot of rejections could indicate you need to > vacuum. "repossessed" is the number of times a strategy buffer was > pinned or in use by another backend and had to be removed from the > strategy ring and replaced with a new shared buffer. This gives you some > indication that there is contention on blocks recently used by a > strategy. I don't immediately see a real use case for repossessed. Why isn't it sufficient to count it as part of rejected? Greetings, Andres Freund
Commits
-
Stabilize pg_stat_io writes test
- ef719e7b32fc 16.0 landed
-
Fix flakey pg_stat_io test
- 1be0fdb9de55 16.0 landed
-
Suppress more compiler warnings in new pgstats code.
- b1b86820d84e 16.0 landed
-
Suppress compiler warnings in new pgstats code.
- 728560db7d86 16.0 landed
-
Add tests for pg_stat_io
- 10a082bf7215 16.0 landed
-
Create regress_tblspc in test_setup
- 9c83bbcf7ef6 16.0 landed
-
Add pg_stat_io view, providing more detailed IO statistics
- a9c70b46dbe1 16.0 landed
-
pgstat: Track more detailed relation IO statistics
- f30d62c2fc60 16.0 landed
-
pgstat: Infrastructure for more detailed IO statistics
- 28e626bde00e 16.0 landed
-
doc: Fix some issues in logical replication section
- 4f74f5641d53 16.0 cited
-
Manual cleanup and pgindent of pgstat and bufmgr related code
- 250c8ee07ede 16.0 landed
-
Have the planner consider Incremental Sort for DISTINCT
- 3c6fc58209f2 16.0 cited
-
Use actual backend IDs in pg_stat_get_backend_idset() and friends.
- d7e39d72ca1c 16.0 cited
-
Remove redundant call to pgstat_report_wal()
- 1bdd54e662d5 16.0 landed
-
Add BackendType for standalone backends
- 0c679464a837 16.0 landed
-
Initialize backend status reporting during bootstrap.
- 3b34645678d1 15.0 landed