Re: [EXTERNAL] Support load balancing in libpq

Jelte Fennema-Nio <postgres@jeltef.nl>

From: Jelte Fennema <postgres@jeltef.nl>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, Andrey Borodin <amborodin86@gmail.com>, Jacob Champion <jchampion@timescale.com>, Maxim Orlov <orlovmg@gmail.com>, Jelte Fennema <Jelte.Fennema@microsoft.com>, Michael Banck <mbanck@gmx.net>, Aleksander Alekseev <aleksander@timescale.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2023-03-27T11:50:39Z
Lists: pgsql-hackers

Attachments

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 vs bool return type. The existing
static libpq functions are a bit of a mixed bag around this, so either
way seems fine to me. And moving the log inside the function seems
fine too. But it seems you accidentally removed the "goto
error_return" part as well, so now we're completely ignoring the
allocation failure. The attached patch fixes that.

>+ok($node1_occurences > 1, "expected at least one execution on node1, found none");
>+ok($node2_occurences > 1, "expected at least one execution on node2, found none");
>+ok($node3_occurences > 1, "expected at least one execution on node3, found none");

I changed the message to be a description of the expected case,
instead of the failure case. This is in line with the way these
messages are used in other tests, and indeed seems like the correct
way because you get output from "meson test -v postgresql:libpq /
libpq/003_load_balance_host_list" like this:
▶ 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 nodes        OK

Finally, I changed a few small typos in your updated commit message
(some of which originated from my earlier commit messages)

Commits

  1. Fix pointer cast for seed calculation on 32-bit systems

  2. Copy and store addrinfo in libpq-owned private memory

  3. libpq: Use modern socket flags, if available.