On Mon, Aug 31, 2015 at 8:01 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.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.
