From: | Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Support worker_spi to execute the function dynamically. |
Date: | 2023-07-20 09:08:04 |
Message-ID: | 46e53055d8d1c92704d3cd2709c722e7@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2023-07-20 13:50, Bharath Rupireddy wrote:
> On Thu, Jul 20, 2023 at 10:09 AM Michael Paquier <michael(at)paquier(dot)xyz>
> wrote:
>>
>> On Thu, Jul 20, 2023 at 09:43:37AM +0530, Bharath Rupireddy wrote:
>> > +1. However, a comment above helps one to understand why some GUCs are
>> > defined before if (!process_shared_preload_libraries_in_progress). As
>> > this is an example extension, it will help understand the reasoning
>> > better. I know we will it in the commit message, but a direct comment
>> > helps:
>> >
>> > /*
>> > * Note that this GUC is defined irrespective of worker_spi shared library
>> > * presence in shared_preload_libraries. It's possible to create the
>> > * worker_spi extension and use functions without it being specified in
>> > * shared_preload_libraries. If we return from here without defining this
>> > * GUC, the dynamic workers launched by worker_spi_launch() will keep
>> > * crashing and restarting.
>> > */
>>
>> WFM to be more talkative here and document things, but I don't think
>> that's it. How about a simple "These GUCs are defined even if this
>> library is not loaded with shared_preload_libraries, for
>> worker_spi_launch()."
>
> LGTM.
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.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Support-worker_spi-to-execute-the-function-dynamical.patch | text/x-diff | 3.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bharath Rupireddy | 2023-07-20 09:08:29 | Re: WAL Insertion Lock Improvements |
Previous Message | Masahiro Ikeda | 2023-07-20 08:54:55 | Re: Support worker_spi to execute the function dynamically. |