Re: Design of pg_stat_subscription_workers vs pgstats

Hi,

On 2022-02-19 09:16:40 -0700, David G. Johnston wrote:
> On Sat, Feb 19, 2022 at 9:02 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > Even leaving everything else aside, a key of (dboid, subid, subrelid),
> > where
> > subrelid can be NULL, but where (dboid, subid) is *not* unique, imo is poor
> > relational design. What is the justification for mixing relation specific
> > and
> > non-relation specific contents in this view?

> The to-be-created pg_subscription_worker_error has this same design.
> It really is a worker-oriented view so the PK should ideally (ignoring
> dboid at the moment) be just workerid and subid and subrelid would just be
> attributes, of which subrelid is optional. But a worker's natural key is
> (subid, subrelid) so long as you accept that null has to be considered
> equal to itself. Not usually an ideal model to pick but it seems like this
> is one of those exceptions to the rule that works just fine. Feel free to
> use InvalidOID instead of null if that makes things more consistent and
> easier to implement. The conceptual model is still the same. It doesn't
> seem to be beneficial to have tablesync and apply workers have their own
> distinct relations. They are similar enough that they can share this one.

I'm not so convinced. "User space" reasonably might want to know why a certain
replication hasn't yet become part of logical replication / why a refresh
hasn't completed yet. For that something like

SELECT *
FROM pg_subscription ps JOIN pg_subscription_worker_error pwe ON (ps.oid = pwe.subid)
WHERE subrelid = 'foo'::regclass;

makes sense.

The apply side is different from that. Yes, a "subrelid = ..." predicate would
filter out non-relation specific failures, but it still seems weird that apply
failures would even need to be filtered out for something like this.

And conversely, monitoring for apply failures is conceptually looking for
something different than tablesync.

IMO the type of information you'd want for apply failures is substantially
different enough from worker failures that I don't really see the temptation
to put them in the same table.

I also still think that _worker shouldn't be part of any of the naming
here. It's an implementation detail that we use one worker for one tablesync
etc. It'd make sense for one apply worker to sync multiple small tables, and
it'd make a lot of sense for multiple apply workers to collaborate on syncing
one large relation.

Greetings,

Andres Freund