On Mon, 15 Apr 2024 at 11:18, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Apr 11, 2024 at 6:58 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
> >
> > While working on [0] i have noticed this comment in
> > TerminateOtherDBBackends function:
> >
> > /*
> > * Check whether we have the necessary rights to terminate other
> > * sessions. We don't terminate any session until we ensure that we
> > * have rights on all the sessions to be terminated. These checks are
> > * the same as we do in pg_terminate_backend.
> > *
> > * In this case we don't raise some warnings - like "PID %d is not a
> > * PostgreSQL server process", because for us already finished session
> > * is not a problem.
> > */
> >
> > This statement is not true after 3a9b18b.
> > "These checks are the same as we do in pg_terminate_backend."
> >
> > But the code is still correct, I assume... or not? In fact, we are
> > killing autovacuum workers which are working with a given database
> > (proc->roleId == 0), which is OK in that case. Are there any other
> > cases when proc->roleId == 0 but we should not be able to kill such a
> > process?
> >
>
> Good question. I am not aware of such cases but I wonder if we should
> add a check similar to 3a9b18b [1] for the reason given in the commit
> message. I have added Noah to see if he has any suggestions on this
> matter.
This function is only for drop database with force option, in case of
drop database with force option there will be a valid database id and
we will not include "logical replication launcher" and "autovacuum
launcher" processes as they will not have any database id. For
"autovacuum workers" that are associated with this database, we will
send the termination signal and then drop the database. As we are not
causing any denial of service attack in this case I felt that could be
the reason the check was not added in this case.
Regards,
Vignesh
