Re: dropdb --force

út 22. 10. 2019 v 5:09 odesílatel Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
napsal:

> On Mon, Oct 21, 2019 at 2:15 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> > po 21. 10. 2019 v 10:25 odesílatel Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> napsal:
> >>
> >>
> >> Sorry, but I am not able to understand the reason. Are you worried
> >> about the comments atop CountOtherDBBackends which says it is used in
> >> Drop Database and related commands?
> >
> >
> > no, just now the code in dropdb looks like
> >
> > if (force)
> > TerminateOtherDBBackends(...);
> >
> > CountOtherDBBackends(...);
> >
> > if I call CountOtherDBBackends from TerminateOtherDBBackends, then code
> will look like
> >
> > if (force)
> > TerminateOtherDBBackends(...);
> > else
> > CountOtherDBBackends(...);
> >
> > That looks like CountOtherDBBackends is not called when force clause is
> active. And this is not true.
> >
>
> Hmm, can't we pass force as a parameter to TerminateOtherDBBackends()
> and then take the decision inside that function? That will make the
> code of dropdb function a bit simpler.
>

I don't think so it is correct. Because without FORCE flag, you should not
to call TeminateOtherDBBackend ever.

Maybe I don't understand what is wrong.

if (force)
terminate();

CountOtherDBBackends()
if (some numbers)
ereport(ERROR, ..

This is fully correct for me.

> > So I prefer current relations between routines.
> >
> >
> >
> >>
> >> > But I can (and I have not any problem with it) remove or
> significantly decrease sleeping time in TerminateOtherDBBackends.
> >> >
> >> > 100 ms is maybe very much - but zero is maybe too low. If there will
> not be any time between TerminateOtherDBBackends and CountOtherDBBackends,
> then probably CountOtherDBBackends hit waiting 100ms.
> >> >
> >> > What about only 5 ms sleeping in TerminateOtherDBBackends?
> >> >
> >>
> >> I am not completely sure about what is the most appropriate thing to
> >> do, but I favor removing sleep from TerminateOtherDBBackends. OTOH,
> >> there is nothing broken with the logic. Anyone else wants to weigh in
> >> here?
> >
> >
> > ok. But when I remove it, should not be better to set waiting in
> CountOtherDBBackends to some smaller number than 100ms?
> >
>
> CountOtherDBBackends is called from other places as well, so I don't
> think it is advisable to change the sleep time in that function.
> Also, I don't want to add a parameter for it. I think you have a
> point that in some cases we might end up sleeping for 100ms when we
> could do with less sleeping time, but I think it is true to some
> extent today as well. I think we can anyway change it in the future
> if there is a problem with the sleep timing, but for now, I think we
> can just call CountOtherDBBackends after sending SIGTERM and call it
> good. You might want to add a futuristic note in the code.
>
>
ok.

I removed sleeping from TerminateOtherDBBackends().

If you want to change any logic there, please, do it without any
hesitations. Maybe I don't see, what you think.

Regards

Pavel

>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>