On Mon, Apr 4, 2022 at 8:05 PM Andres Freund <and...@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.
pgstat-architecture.diff
Description: Binary data
