> On 30 Mar 2023, at 10:21, Daniel Gustafsson wrote:
>
>> On 30 Mar 2023, at 10:00, Julien Rouhaud wrote:
>>
>> On Thu, Mar 30, 2023 at 3:03 PM Daniel Gustafsson wrote:
>>>
On 30 Mar 2023, at 03:48, Hayato Kuroda (Fujitsu)
wrote:
>>>
While checking the buildfarm, I found a f
> On 30 Mar 2023, at 10:00, Julien Rouhaud wrote:
>
> On Thu, Mar 30, 2023 at 3:03 PM Daniel Gustafsson wrote:
>>
>>> On 30 Mar 2023, at 03:48, Hayato Kuroda (Fujitsu)
>>> wrote:
>>
>>> While checking the buildfarm, I found a failure on NetBSD caused by the
>>> added code[1]:
>>
>> Thanks
On Thu, Mar 30, 2023 at 3:03 PM Daniel Gustafsson wrote:
>
> > On 30 Mar 2023, at 03:48, Hayato Kuroda (Fujitsu)
> > wrote:
>
> > While checking the buildfarm, I found a failure on NetBSD caused by the
> > added code[1]:
>
> Thanks for reporting, I see that lapwing which runs Linux (Debian 7, g
> On 30 Mar 2023, at 03:48, Hayato Kuroda (Fujitsu)
> wrote:
> While checking the buildfarm, I found a failure on NetBSD caused by the added
> code[1]:
Thanks for reporting, I see that lapwing which runs Linux (Debian 7, gcc 4.7.2)
has the same error. I'll look into it today to get a fix comm
Dear Daniel, Jelte
Thank you for creating a good feature!
While checking the buildfarm, I found a failure on NetBSD caused by the added
code[1]:
```
fe-connect.c: In function 'libpq_prng_init':
fe-connect.c:1048:11: error: cast from pointer to integer of different size
[-Werror=pointer-to-int-c
I took another couple of looks at this and pushed it after a few small tweaks
to the docs.
--
Daniel Gustafsson
> I think it's fine to remove it. It originated from postmaster.c, where
> I copied the original implementation of libpq_prng_init from.
I agree to remove unlikely macro here.
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
>> "unlikely" macro is used in libpq_prng_init() in the patch. I wonder
>> if the place is really 'hot' to use "unlikely" macro.
>
> I don't think it is, I was thinking to rewrite as the below sketch:
>
> {
> if (pg_prng_strong_seed(&conn->prng_state)))
> return;
>
> /* fallback
I think it's fine to remove it. It originated from postmaster.c, where
I copied the original implementation of libpq_prng_init from.
On Tue, 28 Mar 2023 at 09:22, Daniel Gustafsson wrote:
>
> > On 28 Mar 2023, at 09:16, Tatsuo Ishii wrote:
>
> > "unlikely" macro is used in libpq_prng_init() in t
> On 28 Mar 2023, at 09:16, Tatsuo Ishii wrote:
> "unlikely" macro is used in libpq_prng_init() in the patch. I wonder
> if the place is really 'hot' to use "unlikely" macro.
I don't think it is, I was thinking to rewrite as the below sketch:
{
if (pg_prng_strong_seed(&conn->prng_state)))
Hi,
"unlikely" macro is used in libpq_prng_init() in the patch. I wonder
if the place is really 'hot' to use "unlikely" macro.
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
Hi,
> I guess you didn't set up the hostnames in /etc/hosts as described in
> 004_load_balance_dns.pl. Then it's expected that the loop body isn't
> covered. As discussed upthread, running this test manually is much
> more cumbersome than is desirable, but it's still better than not
> having the t
> > ```
> > if (conn->addr == NULL && conn->naddr != 0)
> > ```
Afaict this is not necessary, since getaddrinfo already returns an
error if the host could not be resolved to any addresses. A quick test
gives me this error:
error: could not translate host name "doesnotexist" to address: Name
or ser
Hi,
> ```
> +ret = store_conn_addrinfo(conn, addrlist);
> +pg_freeaddrinfo_all(hint.ai_family, addrlist);
> +if (ret)
> +goto error_return;/* message already logged */
> ```
> The goto path is not test-covered.
D'oh, this one is fine since store_conn_addrin
Hi,
> So I think it should be:
>
> ```
> if (conn->addr == NULL && conn->naddr != 0)
> ```
>
> [...]
>
> I will take a look at v16 now.
The code coverage could be slightly better.
In v16-0001:
```
+ret = store_conn_addrinfo(conn, addrlist);
+pg_freeaddrinfo_all(hint.ai_family, a
Hi,
> > ▶ 6/6 - received at least one connection on node1 OK
> > ▶ 6/6 - received at least one connection on node2 OK
> > ▶ 6/6 - received at least one connection on node3 OK
> > ▶ 6/6 - received 50 connections across all nodesOK
>
> Good point.
>
> > Finally, I changed a
> On 27 Mar 2023, at 13:50, Jelte Fennema wrote:
>
> Looks good overall. I attached a new version with a few small changes:
>
>> * Changed store_conn_addrinfo to return int like how all the functions
>>dealing with addrinfo does. Also moved the error reporting to inside
>> there
>>whe
Looks good overall. I attached a new version with a few small changes:
> * Changed store_conn_addrinfo to return int like how all the functions
> dealing with addrinfo does. Also moved the error reporting to inside
> there
> where the error happened.
I don't feel strong about the int
> On 17 Mar 2023, at 09:50, Jelte Fennema wrote:
>
>> The documentation lists the modes disabled and random, but I wonder if it's
>> worth expanding the docs to mention that "disabled" is pretty much a round
>> robin load balancing scheme? It reads a bit odd to present load balancing
>> without
Rebased patch after conflicts with bfc9497ece01c7c45437bc36387cb1ebe346f4d2
v14-0001-libpq-Run-pgindent-after-a9e9a9f32b3.patch
Description: Binary data
v14-0002-Refactor-libpq-to-store-addrinfo-in-a-libpq-owne.patch
Description: Binary data
v14-0003-Support-load-balancing-in-libpq.patch
Desc
> The documentation lists the modes disabled and random, but I wonder if it's
> worth expanding the docs to mention that "disabled" is pretty much a round
> robin load balancing scheme? It reads a bit odd to present load balancing
> without a mention of round robin balancing given how common it is
In general I think this feature makes sense (which has been echoed many times
in the thread), and the implementation strikes a good balance of robustness and
simplicity. Reading this I think it's very close to being committable, but I
have a few comments on the patch series:
+sent to the
22 matches
Mail list logo
