CF-3405-review.txt

* pg_stat_session --> "One row per client backend"
  --> should the name indicate this, e.g. pg_stat_client_backend

* Information is scattered between pg_stat_activity and this view, requiring join
  * Example: client_addr, application_name, usename


* Can we define this outside the function pg_stat_get_session?
#define PG_STAT_GET_SESSION_COLS	9


* Why loop over all backends if a pid is specified?
  * Can't we use pgstat_get_local_beentry_by_index(pid)


----
typedef struct PgBackendStatus
{
	/*
	 * To avoid locking overhead, we use the following protocol: a backend
	 * increments st_changecount before modifying its entry, and again after
	 * finishing a modification.  A would-be reader should note the value of
	 * st_changecount, copy the entry into private memory, then check
	 * st_changecount again.  If the value hasn't changed, and if it's even,
	 * the copy is valid; otherwise start over.  This makes updates cheap
	 * while reads are potentially expensive, but that's the tradeoff we want.
---


* PgBackendStatus.st_session --> Can this be a pointer to PgBackendSessionStatus?

* int64 --> uint64?


pg_proc.dat  ===> should be changed by committers only; otherwise frequent conflicts
 * (as per Andrey Borodin: https://youtu.be/vTV8XhWf3mo?t=2765)


------


backend_status.c

* Macros PGSTAT_IS_* do not seem to help a lot, I prefer the original code
  * Specially with PGSTAT_IS_ACTIVE, that has both RUNNING and FASTPATH

* Line 572: Seems like a breaking change:
  beentry->st_backendType == B_BACKEND
  L629-642 can be merged with it by a nested if