Re: shared-memory based stats collector - v69 - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: shared-memory based stats collector - v69
Date
Msg-id CAKFQuwa9Ve-aUkHQn0FqX89anXfNOT6poJQvabBDhKC6OCgTnQ@mail.gmail.com
Whole thread Raw
In response to Re: shared-memory based stats collector - v66  (Andres Freund <andres@anarazel.de>)
Responses Re: shared-memory based stats collector - v69  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Mon, Apr 4, 2022 at 8:05 PM Andres Freund <andres@anarazel.de> wrote:
- added an architecture overview comment to the top of pgstat.c - not sure if
  it makes sense to anybody but me (and perhaps Horiguchi-san)?


I took a look at this, diff attached.  Some typos and minor style stuff, plus trying to bring a bit more detail to the caching mechanism.  I may have gotten it wrong in adding more detail though.

+ * read-only, backend-local, transaction-scoped, hashtable (pgStatEntryRefHash)
+ * in front of the shared hashtable, containing references (PgStat_EntryRef)
+ * to shared hashtable entries. The shared hashtable thus only needs to be
+ * accessed when the PgStat_HashKey is not present in the backend-local hashtable,
+ * or if stats_fetch_consistency = 'none'.

I'm under the impression, but didn't try to confirm, that the pending updates don't use the caching mechanism, but rather add to the shared queue, and so the cache is effectively read-only.  It is also transaction-scoped based upon the GUC and the nature of stats vis-a-vis transactions.

Even before I added the read-only and transaction-scoped I got a bit hung up on reading:
"The shared hashtable only needs to be accessed when no prior reference to the shared hashtable exists."

Thinking in terms of key seems to make more sense than value in this sentence - even if there is a one-to-one correspondence.

The code comment about having per-kind definitions in pgstat.c being annoying is probably sufficient but it does seem like a valid comment to leave in the architecture as well.  Having them in both places seems OK.

I am wondering why there are no mentions to the header files in this architecture, only the .c files.

David J.

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: shared-memory based stats collector - v66
Next
From: Greg Stark
Date:
Subject: Re: [PATCH] Add extra statistics to explain for Nested Loop