Re: Invalidate the subscription worker in cases where a user loses their superuser status

Peter Smith <smithpb2250@gmail.com>

From: Peter Smith <smithpb2250@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-09-28T23:24:52Z
Lists: pgsql-hackers
Some minor review comments for v4-0001:

======
src/backend/replication/logical/worker.c

1.
+ /*
+ * Exit if the owner of the subscription has changed from superuser to a
+ * non-superuser.
+ */
+ if (!newsub->isownersuperuser && MySubscription->isownersuperuser)
+ {
+ if (am_parallel_apply_worker())
+ ereport(LOG,
+ errmsg("logical replication parallel apply worker for subscription
\"%s\" will stop because subscription owner has become non-superuser",
+    MySubscription->name));
+ else
+ ereport(LOG,
+ errmsg("logical replication worker for subscription \"%s\" will
restart because subscription owner has become non-superuser",
+    MySubscription->name));
+
+ apply_worker_exit();
+ }
+

/because subscription owner has become non-superuser/because the
subscription owner has become a non-superuser/  (in 2 places)

======
src/include/catalog/pg_subscription.h

2.
  char    *origin; /* Only publish data originating from the
  * specified origin */
+ bool isownersuperuser; /* Is subscription owner superuser? */
 } Subscription;

~

2a.
Would it be better to put this new field adjacent to the existing
'owner' field, since they kind of belong together?

~

2b.
None of the other bool fields here has an 'is' prefix, so you could
consider a shorter field name, like 'ownersuperuser' or
'superuserowner', etc.

======
Kind Regards,
Peter Smith.
Fujitsu Australia



Commits

  1. Restart the apply worker if the privileges have been revoked.