Re: Add 'worker_type' to pg_stat_subscription

On Fri, Sep 8, 2023 at 8:28 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> On Thu, Sep 07, 2023 at 12:36:29PM +1200, Peter Smith wrote:
> > Modified as suggested. PSA v3.
>
> Thanks. I've attached v4 with a couple of small changes. Notably, I've
> moved the worker_type column to before the pid column, as it felt more
> natural to me to keep the PID columns together. I've also added an
> elog(ERROR, ...) for WORKERTYPE_UNKNOWN, as that seems to be the standard
> practice elsewhere.
> That being said, are we absolutely confident that this
> really cannot happen? I haven't looked too closely, but if there is a
> small race or something that could cause us to see a worker with this type,
> perhaps it would be better to actually list it as "unknown". Thoughts?

The type is only assigned during worker process launch, and during
process cleanup [1]. It's only possible to be UNKNOWN after
logicalrep_worker_cleanup().

AFAIK the stats can never see a worker with an UNKNOWN type, although
it was due to excessive caution against something unforeseen that my
original code did below instead of the elog.

+ case WORKERTYPE_UNKNOWN: /* should not be possible */
+ nulls[9] = true;

Adding "unknown" for something that is supposed to be impossible might
be slight overkill, but so long as there is no obligation to write
about "unknown" in the PG DOCS then I agree it is probably better to
do that,

------
[1] https://github.com/search?q=repo%3Apostgres%2Fpostgres%20%20worker-%3Etype&type=code

Kind Regards,
Peter Smith.
Fujitsu Australia