From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | vignesh C <vignesh21(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Invalidate the subscription worker in cases where a user loses their superuser status |
Date: | 2023-09-28 23:24:52 |
Message-ID: | CAHut+Pua4UP2KxY=4N2TQACkM2Hua=Ay9ba=Z10t5E_1XjMdaA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-09-28 23:59:39 | Re: Requiring recovery.signal or standby.signal when recovering with a backup_label |
Previous Message | Tom Lane | 2023-09-28 23:17:37 | Re: Annoying build warnings from latest Apple toolchain |