Re: Add Option To Check All Addresses For Matching target_session_attr

I updated the patch in accordance with your feedback.

- The variable name has been changed to try_all_addrs, I think this
makes sense as it is shorter and coincides with the variables
try_next_addr and try_next_host even though it doesn't perfectly
convey its relationship to target_session_attrs.
- I fixed the typos and the inaccurate assertion messages in the 008_ test file
- I refactored how the try_all_addrs variable flows in the code to
coincide with how load_balance_type is handled. try_all_attrs contains
the unparsed value while try_all_addrs_type contains the parsed value
of type enum PGTryAddrType. This may be a bit overkill but it
addressed a couple of your bulletpoints above, apologies if I judged
incorrectly here.
- Changed some of the wording in the docs as it was just wrong. Also
changed all references in the docs to reference "try" instead of
"check".

With regards to the correct load balancing behavior I can think of 2 options:
1. randomly choose a host and then randomly choose an address within
that host. If the current addresses session_attr does not match
target_session_attr then move on to next address if any remains in
that host and move onto the next random host if no addresses remain.
2. Resolve all addresses in all hosts and randomly select an address
from that list.

I don't believe that any test cases exist that validate the
functionality of a combination of multiple hosts, multiple address
within each host, and target session attributes. Happy to add test
case coverage over this if it helps get this patch moving.

I was also thinking should try_all_addrs input be 0/1 or a more human
readable disable/enable or both?

On Fri, Mar 6, 2026 at 12:44 AM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
>
>
> > On 16 Aug 2025, at 04:43, Andrew Jackson <andrewjackson947(at)gmail(dot)com> wrote:
> >
> > Attached is the rebased patch.
>
> I've took a look into the patch again.
>
> The behavior and integration with the connection state machine look correct,
> and the tests + docs are in good shape. Some notes:
> 1. Use a dedicated default "0" for check_all_addrs (not DefaultLoadBalanceHosts,
> this one is used for load balancing, need more "0").
> 2. Guard the two strcmp(conn->check_all_addrs, "1") uses so they are safe when
> conn->check_all_addrs is NULL.
> 3. Fix the test typos in 008 (standby_expeect_traffic and the three “on node1”
> messages).
> 4. Parse check_all_addrs once into a bool (like load_balance_type) and use that
> in the connection path for consistency and clarity.
>
> Now about important part: is the name "check_all_addrs" good?
> I've asked LLM after explaining it what the feature does. PFA attached output.
>
> Personally, I like "try_all_addrs".
>
> It's a bit unclear to me how randomization (load balancing) on different
> addresses should work.
>
>
> Best regards, Andrey Borodin.
>