Re: Dependency between bgw_notify_pid and bgw_flags

On Wed, Sep 2, 2015 at 2:02 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Aug 31, 2015 at 8:01 AM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> >> Thanks. It needs testing though to see if it really works as
> >> intended. Can you look into that?
> >
> > PFA the patch containing your code changes + test module. See if that
> meets
> > your expectations.
>
>
PFA patch with improved test module and fix for a bug.

bgworker_sigusr1_handler() should set the latch when set_latch_on_sigusr1
is true, similar to procsignal_sigusr1_handler(). Without this fix, if a
background worker without DATABASE_CONNECTION flag calls
WaitForBackgroundWorker*() functions, those functions wait indefinitely as
the latch is never set upon receipt of SIGUSR1.

> Thanks. I don't think this test module is suitable for commit for a
> number of reasons, including the somewhat hackish use of exit(0)
> instead of proper error reporting

I have changed this part of code.

> , the fact that you didn't integrate
> it into the Makefile structure properly

What was missing? I am able to make {check,clean,install) from the
directory. I can also make -C <dirpath> check from repository's root.

> and the fact that without the
> postmaster.c changes it hangs forever instead of causing a test
> failure.

Changed this too. The SQL level function test_bgwnotify() now errors out if
it doesn't receive notification in specific time.

> But it's sufficient to show that the code changes have the
> intended effect.
>

Looking at the kind of bugs I am getting, we should commit this test
module. Let me know your comments, I will fix those.

> I've committed this and back-patched it to 9.5, but not further. It's
> a bug fix, but it's also a refactoring exercise, so I'd rather not
> push it into released branches.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company