public inbox for [email protected]  

help / color / mirror / Atom feed

From: vignesh C <[email protected]>
To: Amit Kapila <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Invalidate the subscription worker in cases where a user loses their superuser status
Date: Mon, 25 Sep 2023 00:32:03 +0530
Message-ID: <CALDaNm2JDxrsSnX21rnVZNAmZ7xcVE5vb2vrMMnPsU1Xrhz3BA@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1+Dw+0B9vb6rTcPmFWJHyksfu8LJwph1_01f8+C63+DnQ@mail.gmail.com>
References: <CALDaNm2Dxmhq08nr4P6G+24QvdBo_GAVyZ_Q1TcGYK+8NHs9xw@mail.gmail.com>
	<CAA4eK1+Dw+0B9vb6rTcPmFWJHyksfu8LJwph1_01f8+C63+DnQ@mail.gmail.com>

On Sat, 23 Sept 2023 at 11:28, Amit Kapila <[email protected]> wrote:
>
> On Sat, Sep 23, 2023 at 1:27 AM vignesh C <[email protected]> wrote:
> >
> >
> > Fixed this issue by checking if the subscription owner has changed
> > from superuser to non-superuser in case the pg_authid rows changes.
> > The attached patch has the changes for the same.
> >
>
> @@ -3952,7 +3953,9 @@ maybe_reread_subscription(void)
>   newsub->passwordrequired != MySubscription->passwordrequired ||
>   strcmp(newsub->origin, MySubscription->origin) != 0 ||
>   newsub->owner != MySubscription->owner ||
> - !equal(newsub->publications, MySubscription->publications))
> + !equal(newsub->publications, MySubscription->publications) ||
> + (!superuser_arg(MySubscription->owner) &&
> + MySubscription->isownersuperuser))
>   {
>   if (am_parallel_apply_worker())
>   ereport(LOG,
> @@ -4605,6 +4608,13 @@ InitializeLogRepWorker(void)
>   proc_exit(0);
>   }
>
> + /*
> + * Fetch subscription owner is a superuser. This value will be later
> + * checked to see when there is any change with this role and the worker
> + * will be restarted if required.
> + */
> + MySubscription->isownersuperuser = superuser_arg(MySubscription->owner);
>
> Why didn't you filled this parameter in GetSubscription() like other
> parameters? If we do that then the comparison of first change in your
> patch will look similar to all other comparisons.

I felt this variable need not be added to the pg_subscription catalog
table, instead we could save the state of subscription owner when the
worker is started and compare this value during invalidations. As this
information is added only to the memory Subscription structure and not
added to the catalog FormData_pg_subscription, the checking is
slightly different in this case. Also since this variable will be used
only within the worker, I felt we need not add it to the catalog.

Regards,
Vignesh

view thread (28+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Invalidate the subscription worker in cases where a user loses their superuser status
  In-Reply-To: <CALDaNm2JDxrsSnX21rnVZNAmZ7xcVE5vb2vrMMnPsU1Xrhz3BA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox