Horton

Re: shared-memory based stats collector - v66

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: melanieplageman@gmail.com, ibrar.ahmad@gmail.com, masao.fujii@oss.nttdata.com, gkokolatos@protonmail.com, pgsql-hackers@lists.postgresql.org
Date: 2022-04-02T08:16:48Z
Lists: pgsql-hackers 
Hi,

On 2022-03-25 17:24:18 +0900, Kyotaro Horiguchi wrote:
> > AFIXME: Isn't PGSTAT_MIN_INTERVAL way too long? What is the justification
> > for increasing it?
> 
> It is 1000ms in the comment just above but actually 10000ms. The
> number came from a discussion that if we have 1000 clients and each
> backend writes stats once per 0.5 seconds, totally we flush pending
> data to shared area at 2000 times per second which is too frequent. I
> raised it to 5000ms, then 10000ms.  So the expected maximum flush
> frequency is reduces to 100 times per second.  Of course it is
> assuming the worst case and the 10000ms is apparently too long for the
> average cases.
> 
> The current implement of pgstat postpones flushing if lock collision
> happens then postpone by at most 60s.  This is a kind of
> auto-averaging mechanishm.  It might be enough and we can reduce the
> PGSTAT_MIN_INTERVAL to 500ms or so.

I just noticed that the code doesn't appear to actually work like that right
now. Whenever the timeout is reached, pgstat_report_stat() is called with
force = true.

And even if the backend is busy running queries, once there's contention, the
next invocation of pgstat_report_stat() will return the timeout relative to
pendin_since, which then will trigger a force report via a very short timeout
soon.

It might actually make sense to only ever return PGSTAT_RETRY_MIN_INTERVAL
(with a slightly different name) from pgstat_report_stat() when blocked
(limiting the max reporting delay for an idle connection) and to continue
calling pgstat_report_stat(force = true).  But to only trigger force
"internally" in pgstat_report_stat() when PGSTAT_MAX_INTERVAL is reached.

I think that'd mean we'd report after max PGSTAT_RETRY_MIN_INTERVAL in an idle
connection, and try reporting every PGSTAT_RETRY_MIN_INTERVAL (increasing up
to PGSTAT_MAX_INTERVAL when blocked) on busy connections.

Makes sense?


I think we need to do something with the pgstat_report_stat() calls outside of
postgres.c. Otherwise there's nothing limiting their reporting delay, because
they don't have the timeout logic postgres.c has.  None of them is ever hot
enough to be problematic, so I think we should just make them pass force=true?

Greetings,

Andres Freund

Commits

  1. Improve comment about dropped entries in pgstat.c

  2. Fix temporary memory leak in system table index scans

  3. pgstat: set timestamps of fixed-numbered stats after a crash.

  4. pgstat: Update docs to match the shared memory stats reality.

  5. pgstat: Hide instability in stats.spec with -DCATCACHE_FORCE_RELEASE.

  6. pgstat: add/extend tests for resetting various kinds of stats.

  7. Add minimal tests for recovery conflict handling.

  8. pgstat: test stats interactions with physical replication.

  9. pgstat: add tests for handling of restarts, including crashes.

  10. pgstat: add tests for transaction behaviour, 2PC, function stats.

  11. pgstat: add pg_stat_have_stats() test helper.

  12. pgstat: add pg_stat_force_next_flush(), use it to simplify tests.

  13. pgstat: move pgstat.c to utils/activity.

  14. pgstat: store statistics in shared memory.

  15. pgstat: remove stats_temp_directory.

  16. pgstat: rename STATS_COLLECTOR GUC group to STATS_CUMULATIVE.

  17. pgstat: revise replication slot API in preparation for shared memory stats.

  18. pgstat: scaffolding for transactional stats creation / drop.

  19. pgstat: introduce PgStat_Kind enum.

  20. pgstat: prepare APIs used by pgstatfuncs for shared memory stats.

  21. pgstat: add pgstat_copy_relation_stats().

  22. pgstat: rename some pgstat_send_* functions to pgstat_report_*.

  23. pgstat: stats collector references in comments.

  24. pgstat: move transactional code into pgstat_xact.c.

  25. dsm: allow use in single user mode.

  26. dshash: revise sequential scan support.

  27. pgstat: remove some superflous comments from pgstat.h.

  28. pgstat: reorder pgstat.[ch] contents.

  29. pgstat: split different types of stats into separate files.

  30. pgstat: introduce pgstat_relation_should_count().

  31. pgstat: separate "xact level" handling out of relation specific functions.

  32. pgstat: rename pgstat_initstats() to pgstat_relation_init().

  33. pgstat: split out WAL handling from pgstat_{initialize,report_stat}.

  34. pgstat: run pgindent on pgstat.c/h.

  35. pgstat: split relation, database handling out of pgstat_report_stat().

  36. Move code around in StartupXLOG().

  37. pgstat: Prepare to use mechanism for truncated rels also for droppped rels.

  38. pgstat: Split out relation stats handling from AtEO[Sub]Xact_PgStat() etc.

  39. pgstat: Schedule per-backend pgstat shutdown via before_shmem_exit().

  40. Schedule ShutdownXLOG() in single user mode using before_shmem_exit().

  41. Make parallel worker shutdown complete entirely via before_shmem_exit().

  42. pgstat: Bring up pgstat in BaseInit() to fix uninitialized use of pgstat by AV.

  43. pgstat: split reporting/fetching of bgwriter and checkpointer stats.

  44. Split backend status and progress related functionality out of pgstat.c.

  45. Split wait event related code from pgstat.[ch] into wait_event.[ch].

  46. Make archiver process an auxiliary process.

  47. Force to send remaining WAL stats to the stats collector at walwriter exit.

  48. Add pg_stat_database counters for sessions and session time

  49. Collect statistics about SLRU caches

  50. Don't run atexit callbacks in quickdie signal handlers.

  51. Create a "fast path" for acquiring weak relation locks.