Re: SLRU statistics
От | Tomas Vondra |
---|---|
Тема | Re: SLRU statistics |
Дата | |
Msg-id | 20200229122441.ow7gnnqgesmojyc3@development обсуждение исходный текст |
Ответ на | Re: SLRU statistics (Alvaro Herrera <alvherre@2ndquadrant.com>) |
Ответы |
Re: SLRU statistics
|
Список | pgsql-hackers |
On Fri, Feb 28, 2020 at 08:19:18PM -0300, Alvaro Herrera wrote: >On 2020-Jan-21, Tomas Vondra wrote: > >> On Tue, Jan 21, 2020 at 05:09:33PM +0900, Masahiko Sawada wrote: > >> > I've not tested the performance impact but perhaps we might want to >> > disable these counter by default and controlled by a GUC. And similar >> > to buffer statistics it might be better to inline >> > pgstat_count_slru_page_xxx function for better performance. >> >> Hmmm, yeah. Inlining seems like a good idea, and maybe we should have >> something like track_slru GUC. > >I disagree with adding a GUC. If a performance impact can be measured >let's turn the functions to static inline, as already proposed. My >guess is that pgstat_count_slru_page_hit() is the only candidate for >that; all the other paths involve I/O or lock acquisition or even WAL >generation, so the impact won't be measurable anyhow. We removed >track-enabling GUCs years ago. > Did we actually remove track-enabling GUCs? I think we still have - track_activities - track_counts - track_io_timing - track_functions But maybe I'm missing something? That being said, I'm not sure we need to add a GUC. I'll do some measurements and we'll see. Maybe the statis inline will me enough. >BTW, this comment: > /* update the stats counter of pages found in shared buffers */ > >is not strictly true, because we don't use what we normally call "shared >buffers" for SLRUs. > Oh, right. Will fix. >Patch applies cleanly. I suggest to move the page_miss() call until >after SlruRecentlyUsed(), for consistency with the other case. > OK. >I find SlruType pretty odd, and the accompanying "if" list in >pg_stat_get_slru() correspondingly so. Would it be possible to have >each SLRU enumerate itself somehow? Maybe add the name in SlruCtlData >and query that, somehow. (I don't think we have an array of SlruCtlData >anywhere though, so this might be a useless idea.) > Well, maybe. We could have a system to register SLRUs dynamically, but the trick here is that by having a fixed predefined number of SLRUs simplifies serialization in pgstat.c and so on. I don't think the "if" branches in pg_stat_get_slru() are particularly ugly, but maybe we could replace the enum with a registry of structs, something like rmgrlist.h. It seems like an overkill to me, though. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
В списке pgsql-hackers по дате отправления: