public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bharath Rupireddy <[email protected]>
To: Masahiro Ikeda <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: [email protected]
Subject: Re: Support worker_spi to execute the function dynamically.
Date: Thu, 20 Jul 2023 15:44:12 +0530
Message-ID: <CALj2ACV0fr-9OTZqtWu-9XrtUbBZCYb0UK859gfvJ0oqm3YvUA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<ZLiwP5vrT/[email protected]>
<CALj2ACVOCiXirZFRL-gi7mV2zgdH4e3-vyJQx2fxdzycJTv3qg@mail.gmail.com>
<[email protected]>
<CALj2ACWj7qF-YYVi05okBFJnPugkwsM8PSrEwfHs-tQqKO80Uw@mail.gmail.com>
<[email protected]>
On Thu, Jul 20, 2023 at 2:38 PM Masahiro Ikeda <[email protected]> wrote:
>
> Thanks for discussing about the patch. I updated the patch from your
> comments
> * v2-0001-Support-worker_spi-to-execute-the-function-dynamical.patch
>
> I found another thing to be changed better. Though the tests was assumed
> "shared_preload_libraries = worker_spi", the background workers failed
> to
> be launched in initialized phase because the database is not created
> yet.
>
> ```
> # make check # in src/test/modules/worker_spi
> # cat log/postmaster.log # in src/test/modules/worker_spi/
> 2023-07-20 17:58:47.958 JST worker_spi[853620] FATAL: database
> "contrib_regression" does not exist
> 2023-07-20 17:58:47.958 JST worker_spi[853621] FATAL: database
> "contrib_regression" does not exist
> 2023-07-20 17:58:47.959 JST postmaster[853612] LOG: background worker
> "worker_spi" (PID 853620) exited with exit code 1
> 2023-07-20 17:58:47.959 JST postmaster[853612] LOG: background worker
> "worker_spi" (PID 853621) exited with exit code 1
> ```
>
> It's better to remove "shared_preload_libraries = worker_spi" from the
> test configuration. I misunderstood that two background workers would
> be launched and waiting at the start of the test.
I don't think that change is correct. The worker_spi essentially shows
how to start bg workers with RegisterBackgroundWorker and dynamic bg
workers with RegisterDynamicBackgroundWorker. If
shared_preload_libraries = worker_spi not specified in there, you will
miss to start RegisterBackgroundWorkers. Is giving an initidb time
database name to worker_spi.database work there? If the database for
bg workers doesn't exist, changing bgw_restart_time from
BGW_NEVER_RESTART to say 1 will help to see bg workers coming up
eventually.
I think it's worth adding test cases for the expected number of bg
workers (after creating worker_spi extension) and dynamic bg workers
(after calling worker_spi_launch()). Also, to distinguish bg workers
and dynamic bg workers, you can change
bgw_type in worker_spi_launch to "worker_spi dynamic worker".
- /* get the configuration */
+ /* Get the configuration */
- /* set up common data for all our workers */
+ /* Set up common data for all our workers */
These unrelated changes better be there as-is. Because, the postgres
code has both commenting styles /* Get .... */ or /* get ....*/, IOW,
single line comments starting with both uppercase and lowercase.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
view thread (18+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Support worker_spi to execute the function dynamically.
In-Reply-To: <CALj2ACV0fr-9OTZqtWu-9XrtUbBZCYb0UK859gfvJ0oqm3YvUA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox