Horton

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

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Magnus Hagander <magnus@hagander.net>, Pg Hackers <pgsql-hackers@postgresql.org>, Lukas Fittl <lukas@fittl.com>, Thomas Munro <thomas.munro@gmail.com>
Date: 2021-12-15T22:38:52Z
Lists: pgsql-hackers 
On Fri, Dec 03, 2021 at 03:02:24PM -0500, Melanie Plageman wrote:
> Thanks again! I really appreciate the thorough review.
> 
> I have combined responses to all three of your emails below.
> Let me know if it is more confusing to do it this way.

One email is better than three - I'm just not a model citizen ;)

Thanks for updating the patch.  I checked that all my previous review comments
were addressed (except for the part about passing the 3D array to a function -
I know that technically the pointer is being passed).

+int backend_type_get_idx(BackendType backend_type)                                                                                                                                               
+BackendType idx_get_backend_type(int idx)                                                                                                                                                        

=> I think it'd be desirable for these to be either static functions (which
won't work for your needs) or macros, or inline functions in the header.

-       if (strcmp(target, "archiver") == 0)                                                                                                                                                      
+       pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_RESETSHAREDCOUNTER);                                                                                                                            
+       if (strcmp(target, "buffers") == 0)                                                                                                                                                       

=> This should be added in alphabetical order.  Which is unimportant, but it
will also makes the patch 2 lines shorter.  The doc patch should also be in
order.

+                * Don't count dead backends. They will be added below There are no                                                                                                               

=> Missing a period.

-- 
Justin

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.