Re: Session WAL activity

Hi,

On 2019-12-20 16:38:32 -0500, Bruce Momjian wrote:
> On Thu, Dec 12, 2019 at 09:31:22AM +0800, Craig Ringer wrote:
> > On Fri, 6 Dec 2019 at 09:57, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >
> > On Thu, Dec 05, 2019 at 12:23:40PM +0300, Konstantin Knizhnik wrote:
> > > Concerning keeping PGPROC size as small as possible, I agree that it is
> > > reasonable argument.
> > > But even now it is very large (816 bytes) and adding extra 8 bytes will
> > > increase it on less than 1%.
> >
> > It does not mean that we should add all kind of things to PGPROC as
> > that's a structure sensitive enough already.

Well, but we don't keep other stats in PGPROC, even when we have them in
shared memory? It seems like PgBackendStatus or such might be a better
place?

> > Right. It's not as critical as PGXACT, but PGPROC is still significant for
> > scalability and connection count limits.
> >
> > It's a shame we can't really keep most of it in backend-private memory and copy
> > it to requestors when they want it, say into a temporary DSA or over
> > a shm_mq.

I don't understand what that would buy? Commonly accessed field are just
going to be in L2 or such, with the cacheline being in
modified/exclusive state. The problem isn't that fields / cachelines
*can* be accessed by other backends, it's only a problem *if* they're
frequently accessed. And even if accessed by multiple backends, it's
only really a problem if there are multiple fields *and* they're also
modified (otherwise they can just stay in shared stated across
cpus/sockets).

There *is* an argument for grouping fields in PGPROC by their access
patterns. E.g. something like ->procArrayGroup* is a lot more commonly
accessed by different backends than e.g. this proposed field.

> > But our single threaded model means we just cannot rely on backends being
> > responsive in a timely enough manner to supply data on-demand. That doesn't
> > mean we have to push it to PGPROC though: we could be sending the parts that
> > don't have to be super-fresh to the stats collector or a new related process
> > for active session stats and letting it aggregate them.

We should definitely *NOT* do that. Ferrying things through the stats
collector is really expensive, and everyone pays the price for an
increase in size, not just code accessing the field. In fact, no
reasonable quantity that's known at server start should ever go through
a mechanism as expensive as pgstat - the only reason it exists is that
the number of tables obviously can grow over time.

There's a thread somewhere about a patchset to move all of pgstat into
dynamic shared memory, actually. Because the writes / reads needed by
pgstat are really bad on some systems.

> > That's way beyond the scope of this patch though. So personally I'm OK with the
> > new PGPROC field. Visibility into Pg's activity is woefully limited and
> > something we need to prioritize more.
>
> Uh, how much does the new field get us near the CPU cache line max size
> for a single PGPROC entry?

It's like ~13x the common size of a cache line (64bytes).

Greetings,

Andres Freund