> 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
Rebased
On Tue, 14 Mar 2023 at 19:05, Gregory Stark (as CFM)
wrote:
>
> The pgindent run in b6dfee28f is causing this patch to need a rebase
> for the cfbot to apply it.
v12-0002-Refactor-libpq-to-store-addrinfo-in-a-libpq-owne.patch
Description: Binary data
v12-0003-Support-load-balancing-in
The pgindent run in b6dfee28f is causing this patch to need a rebase
for the cfbot to apply it.
Small update. Improved some wording in the docs.
On Fri, 3 Mar 2023 at 15:37, Jelte Fennema wrote:
>
> > I want to note that the Fisher-Yates algorithm is implemented in a
> > difficult to understand manner.
> > +if (j < i) /* avoid fetching undefined data if j=i */
> > This stuff does not make s
> I want to note that the Fisher-Yates algorithm is implemented in a
> difficult to understand manner.
> +if (j < i) /* avoid fetching undefined data if j=i */
> This stuff does not make sense in case of shuffling arrays inplace. It
> is important only for making a new copy of an array and only in
On Wed, Mar 1, 2023 at 12:03 PM Jelte Fennema wrote:
>
> done and updated cf entry
>
Hi Jelte!
I've looked into the patch. Although so many improvements can be
suggested, It definitely makes sense as-is too.
These improvements might be, for example, sorting hosts according to
ping latency or som
done and updated cf entry
On Wed, 1 Mar 2023 at 20:13, Greg S wrote:
>
> This patch seems to need a rebase.
>
> I'll update the status to Waiting on Author for now. After rebasing
> please update it to either Needs Review or Ready for Committer
> depending on how simple the rebase was and whether
This patch seems to need a rebase.
I'll update the status to Waiting on Author for now. After rebasing
please update it to either Needs Review or Ready for Committer
depending on how simple the rebase was and whether there are open
questions to finish it.
After discussing this patch privately with Andres here's a new version of this
patch. The major differences are:
1. Use the pointer value of the connection as a randomness source
2. Use more precise time as randomness source
3. Move addrinfo changes into a separate commit. This is both to make
the
As far as I can tell this is ready for committer feedback now btw. I'd
really like to get this into PG16.
> It hadn't been my intention to block the patch on it, sorry. Just
> registering a preference.
No problem. I hadn't looked into the shared PRNG solution closely
enough to determine if I thou
On Fri, Jan 13, 2023 at 10:44 AM Jacob Champion wrote:
> And my thought was that the one-time
> initialization could be moved to a place that doesn't need to know the
> connection options at all, to make it easier to reason about the
> architecture. Say, next to the WSAStartup machinery.
(And aft
On Fri, Jan 13, 2023 at 9:10 AM Jelte Fennema wrote:
>
> > Just a quick single-issue review, but I agree with Maxim that having
> > one PRNG, seeded once, would be simpler
>
> I don't agree that it's simpler. Because now there's a mutex you have
> to manage, and honestly cross-platform threading i
> Just a quick single-issue review, but I agree with Maxim that having
> one PRNG, seeded once, would be simpler
I don't agree that it's simpler. Because now there's a mutex you have
to manage, and honestly cross-platform threading in C is not simple.
However, I attached two additional patches tha
On Wed, Sep 14, 2022 at 7:54 AM Maxim Orlov wrote:
> For the patch itself, I think it is better to use a more precise time
> function in libpq_prng_init or call it only once.
> Thought it is a special corner case, imagine all the connection attempts at
> first second will be seeded with the save
Attached an updated patch which should address your feedback and
I updated the commit message.
> I wonder whether making the parameter a boolean will paint us into a
> corner
I made it a string option, just like target_session_attrs. I'm pretty sure a
round-robin load balancing policy could be i
6 +0200
> Subject: [PATCH v5] Support load balancing in libpq
>
> Load balancing connections across multiple read replicas is a pretty
> common way of scaling out read queries. There are two main ways of doing
> so, both with their own advantages and disadvantages:
> 1. Load balanc
Attached is a new version with the tests cleaned up a bit (more comments
mostly).
@Michael, did you have a chance to look at the last version? Because I feel
that the
patch is pretty much ready for a committer to look at, at this point.
v5-0001-Support-load-balancing-in-libpq.patch
Description
I attached a new patch which does the following:
1. adds tap tests
2. adds random_seed parameter to libpq (required for tap tests)
3. frees conn->loadbalance in freePGConn
4. add more expansive docs on the feature its behaviour
Apart from bike shedding on the name of the option I think it's pretty
Hi,
On Mon, Sep 12, 2022 at 02:16:56PM +, Jelte Fennema wrote:
> Attached is an updated patch with the following changes:
> 1. rebased (including solved merge conflict)
> 2. fixed failing tests in CI
> 3. changed the commit message a little bit
> 4. addressed the two remarks from Micheal
> 5.
Hi,
On Wed, Sep 14, 2022 at 05:53:48PM +0300, Maxim Orlov wrote:
> > Also, IMO, the solution must have a fallback mechanism if the
> > standby/chosen host isn't reachable.
>
> Yeah, I think it should. I'm not insisting on a particular name of options
> here, but in my view, the overall idea may b
+1 for overall idea of load balancing via random host selection.
For the patch itself, I think it is better to use a more precise time
function in libpq_prng_init or call it only once.
Thought it is a special corner case, imagine all the connection attempts at
first second will be seeded with the
Attached is an updated patch with the following changes:
1. rebased (including solved merge conflict)
2. fixed failing tests in CI
3. changed the commit message a little bit
4. addressed the two remarks from Micheal
5. changed the prng_state from a global to a connection level value for
thread-saf
Hi,
the patch no longer applies cleanly, please rebase (it's trivial).
I don't like the provided commit message very much, I think the
discussion about pgJDBC having had load balancing for a while belongs
elsewhere.
On Wed, Jun 22, 2022 at 07:54:19AM +, Jelte Fennema wrote:
> I tried to stay
Dear Jelte,
> With plain Postgres this assumption is probably correct. But the main reason
> I'm interested in this patch was because I would like to be able to load
> balance across the workers in a Citus cluster. These workers are all
> primaries.
> Similar usage would likely be possible with B
> we can assume that one of members is a primary and others are secondary.
With plain Postgres this assumption is probably correct. But the main reason
I'm interested in this patch was because I would like to be able to load
balance across the workers in a Citus cluster. These workers are all prim
Dear Jelte,
I like your idea. But do we have to sort randomly even if target_session_attr
is set to 'primary' or 'read-write'?
I think this parameter can be used when all listed servers have same data,
and we can assume that one of members is a primary and others are secondary.
In this case use
> I'm quoting a previous attempt by Satyanarayana Narlapuram on this
> topic [1], it also has a patch set.
Thanks for sharing that. It's indeed a different approach to solve the
same problem. I think my approach is much simpler, since it only
requires minimal changes to the libpq client and none
On Fri, Jun 10, 2022 at 10:01 PM Jelte Fennema
wrote:
>
> Load balancing connections across multiple read replicas is a pretty
> common way of scaling out read queries. There are two main ways of doing
> so, both with their own advantages and disadvantages:
> 1. Load balancing at the client level
I tried to stay in line with the naming of this same option in JDBC and
Npgsql, where it's called "loadBalanceHosts" and "Load Balance Hosts"
respectively. So, actually to be more in line it should be the option for
libpq should be called "load_balance_hosts" (not "loadbalance" like
in the previo
Hi Jelte,
> Load balancing connections across multiple read replicas is a pretty
> common way of scaling out read queries. There are two main ways of doing
> so, both with their own advantages and disadvantages:
> 1. Load balancing at the client level
> 2. Load balancing by connecting to an interm
Load balancing connections across multiple read replicas is a pretty
common way of scaling out read queries. There are two main ways of doing
so, both with their own advantages and disadvantages:
1. Load balancing at the client level
2. Load balancing by connecting to an intermediary load balancer
52 matches
Mail list logo
