Fix a comment in worker.c
Hi,
While reading the code, I realized that the second sentence of the
following comment in worker.c is not correct:
/*
* Exit if the subscription was disabled. This normally should not happen
* as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
*/
if (!newsub->enabled)
{
ereport(LOG,
(errmsg("logical replication apply worker for
subscription \"%s\" will "
"stop because the subscription was disabled",
MySubscription->name)));
proc_exit(0);
}
IIUC the apply worker normally exits here when the subscription is
disabled since we don't stop the apply worker during ALTER
SUBSCRIPTION DISABLE. I've attached a patch to remove it.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
Attachments:
fix_comment_in_worker.c.patchapplication/octet-stream; name=fix_comment_in_worker.c.patchDownload
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index d77bb32bb9..9f2ac45d3c 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2933,10 +2933,7 @@ maybe_reread_subscription(void)
proc_exit(0);
}
- /*
- * Exit if the subscription was disabled. This normally should not happen
- * as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
- */
+ /* Exit if the subscription was disabled */
if (!newsub->enabled)
{
ereport(LOG,
On Wed, Feb 16, 2022 at 5:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
While reading the code, I realized that the second sentence of the
following comment in worker.c is not correct:/*
* Exit if the subscription was disabled. This normally should not happen
* as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
*/
if (!newsub->enabled)
{
ereport(LOG,
(errmsg("logical replication apply worker for
subscription \"%s\" will "
"stop because the subscription was disabled",
MySubscription->name)));proc_exit(0);
}IIUC the apply worker normally exits here when the subscription is
disabled since we don't stop the apply worker during ALTER
SUBSCRIPTION DISABLE. I've attached a patch to remove it.
Yes, I also have the same understanding. Your patch LGTM. I'll push
this unless someone thinks otherwise.
--
With Regards,
Amit Kapila.
On Wed, Feb 16, 2022 at 4:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Wed, Feb 16, 2022 at 5:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
While reading the code, I realized that the second sentence of the
following comment in worker.c is not correct:/*
* Exit if the subscription was disabled. This normally should not happen
* as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
*/
if (!newsub->enabled)
{
ereport(LOG,
(errmsg("logical replication apply worker for
subscription \"%s\" will "
"stop because the subscription was disabled",
MySubscription->name)));proc_exit(0);
}IIUC the apply worker normally exits here when the subscription is
disabled since we don't stop the apply worker during ALTER
SUBSCRIPTION DISABLE. I've attached a patch to remove it.Yes, I also have the same understanding. Your patch LGTM. I'll push
this unless someone thinks otherwise.
Pushed.
--
With Regards,
Amit Kapila.
On Fri, Feb 18, 2022 at 1:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Wed, Feb 16, 2022 at 4:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Wed, Feb 16, 2022 at 5:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
While reading the code, I realized that the second sentence of the
following comment in worker.c is not correct:/*
* Exit if the subscription was disabled. This normally should not happen
* as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
*/
if (!newsub->enabled)
{
ereport(LOG,
(errmsg("logical replication apply worker for
subscription \"%s\" will "
"stop because the subscription was disabled",
MySubscription->name)));proc_exit(0);
}IIUC the apply worker normally exits here when the subscription is
disabled since we don't stop the apply worker during ALTER
SUBSCRIPTION DISABLE. I've attached a patch to remove it.Yes, I also have the same understanding. Your patch LGTM. I'll push
this unless someone thinks otherwise.Pushed.
Thanks!
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/