public inbox for [email protected]  
help / color / mirror / Atom feed
From: Justin Pryzby <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Lukas Fittl <[email protected]>
Cc: Thomas Munro <[email protected]>
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: Wed, 15 Dec 2021 16:38:52 -0600
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU+ECH4tNwGFNERkZA@mail.gmail.com>
References: <[email protected]>
	<CAAKRu_YJMHJ66-ZB86zFCH=Cq4+5w2gnJ14bu8oyZec0C39Dcg@mail.gmail.com>
	<[email protected]>
	<CAAKRu_Yeg+vh6SHNEo1+=O7e-BPX35cU0XQM=YwQRnkFyv_y+w@mail.gmail.com>
	<[email protected]>
	<CAAKRu_bCavNULX2BUM1Ankr=Hc5ySbmMJCGnHCFZBbM=yMvFLA@mail.gmail.com>
	<[email protected]>
	<CAAKRu_ZLE50yXXHTBEjddLfvR9fw=S79Thjzd_yCfvzkDmA7CA@mail.gmail.com>
	<[email protected]>
	<CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU+ECH4tNwGFNERkZA@mail.gmail.com>

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





view thread (108+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox