From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Muhammad Usama <m(dot)usama(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: A new function to wait for the backend exit after termination |
Date: | 2020-12-02 08:30:42 |
Message-ID: | CALj2ACWVer9PO3BGNPiWgu7LoO74F4UU+cgDnZchLkjP4LkPSw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Nov 30, 2020 at 8:10 PM Muhammad Usama <m(dot)usama(at)gmail(dot)com> wrote:
>
> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: tested, passed
> Spec compliant: tested, passed
> Documentation: not tested
>
> I have tested the patch against current master branch (commit:6742e14959a3033d946ab3d67f5ce4c99367d332)
> Both functions work without a problem and as expected.
>
Thanks!
>
> Just a tiny comment/suggestion.
> specifying a -ve timeout in pg_terminate_backed rightly throws an error,
> I am not sure if it would be right or a wrong approach but I guess we can ignore -ve
> timeout in pg_terminate_backend function when wait (second argument) is false.
>
> e.g. pg_terminate_backend(12320, false,-1); -- ignore -1 timout since wait is false
>
IMO, that's not a good idea. I see it this way, for any function first
the input args have to be validated. If okay, then follows the use of
those args and the main functionality. I can also see pg_promote(),
which first does the input timeout validation throwing error if it is
<= 0.
We can retain the existing behaviour.
>
> The new status of this patch is: Ready for Committer
>
Thanks!
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Erik Rijkers | 2020-12-02 09:39:14 | wrong link in acronyms.sgml |
Previous Message | Amit Kapila | 2020-12-02 08:28:44 | Re: [HACKERS] logical decoding of two-phase transactions |