From: Andrey Borodin <x4mmm@yandex-team.ru>
Message-Id: <817CE6D9-C5E3-4625-B001-4515C72A1DD8@yandex-team.ru>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_81011C1F-7E90-4DBF-8676-9005F0D8443B"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: Add Option To Check All Addresses For Matching
 target_session_attr
Date: Fri, 6 Mar 2026 11:44:20 +0500
In-Reply-To: 
 <CAKK5BkHvU1iFXMiexSgu=Wz4A9TtL0ey5teNNTdy4BR+aZFmYw@mail.gmail.com>
Cc: Navrotskiy Artem <bozaro@yandex.ru>,
 Vladimir Sitnikov <sitnikov.vladimir@gmail.com>,
 Dave Page <dpage@pgadmin.org>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
 Evgeny Kuzin <evgeny.kuzin@outlook.com>
To: Andrew Jackson <andrewjackson947@gmail.com>
References: 
 <CAKK5BkESSc69sp2TiTWHvvOHCUey0rDWXSrR9pinyRqyfamUYg@mail.gmail.com>
 <F1096CFA-CF22-4B47-A10B-D1F837BF7AF1@yandex-team.ru>
 <235381750793454@mail.yandex.ru>
 <CAKK5BkHvU1iFXMiexSgu=Wz4A9TtL0ey5teNNTdy4BR+aZFmYw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/817CE6D9-C5E3-4625-B001-4515C72A1DD8%40yandex-team.ru>
Precedence: bulk


--Apple-Mail=_81011C1F-7E90-4DBF-8676-9005F0D8443B
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On 16 Aug 2025, at 04:43, Andrew Jackson <andrewjackson947@gmail.com> =
wrote:
>=20
> 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,=20
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 =
=E2=80=9Con node1=E2=80=9D
   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.


--Apple-Mail=_81011C1F-7E90-4DBF-8676-9005F0D8443B
Content-Disposition: attachment;
	filename=check_all_addrs_variations.txt
Content-Type: text/plain;
	x-unix-mode=0644;
	name="check_all_addrs_variations.txt"
Content-Transfer-Encoding: quoted-printable

On naming and how to configure the feature:

What the option does
It only matters when target_session_attrs is set. On a =
target_session_attrs
mismatch (e.g. we wanted primary but got standby), it controls:
0 (default): treat this host as failed and move to the next host.
1: stay on this host and try the next address for the same hostname.
So the name is really answering: on session-attrs mismatch, try next =
address
vs next host?

Is "check_all_addrs" good?
Pros: Short, easy to type, and the env var PGCHECKALLADDRS is clear.
Cons:
It doesn't mention target_session_attrs, so the link to that feature
isn't obvious.
"Check" is vague (check for what?). The important part is "try all
addresses (for this host) on mismatch."
With only check_all_addrs=3D1 in the conninfo, someone might not guess
it's about session attrs.
So it's usable but not ideal.

Naming alternatives
try_all_addrs
Shorter; "try" is more accurate than "check". Still doesn't tie to
target_session_attrs.
try_next_addr_on_mismatch
Very clear. Long; "mismatch" is a bit generic.
target_session_attrs_try_all_addrs
Explicitly tied to the feature. Too long for conninfo.
session_attrs_try_all_addrs
Shorter, still linked to session attrs. Slightly ambiguous ("try all"
what?).
try_all_host_addrs

Makes "addresses of current host" clear. Doesn't mention session attrs.
A reasonable compromise is try_all_addrs: same length as now, clearer =
verb,
and the docs can state it applies when target_session_attrs is used. If =
you
want the link to session attrs in the name, session_attrs_try_all_addrs =
is
the next best, at the cost of length.
Other ways to configure it
1) No new option (behavioral change only)
Evgeny's approach: always try next address on session-attrs mismatch. No
naming issue, but it changes default behavior and was argued against for
backward compatibility.
2) Fold into target_session_attrs
E.g. target_session_attrs=3Dread-write,try_all_addrs or =
read-write+try_all.
Pro: option is clearly scoped to that feature. Con: one parameter does =
two
things (desired role + connection strategy); parsing and docs get more
complex.
3) Keep a separate option (current design)
A dedicated connection parameter that only has effect when
target_session_attrs is set. Pro: simple parsing, clear in conninfo, =
easy
to document. Con: the name should make the "only with =
target_session_attrs"
contract obvious (in name or docs).
4) Tie to load_balance_hosts
E.g. when load_balance_hosts=3Drandom (or disable) and =
target_session_attrs
is set, add a third mode like try_all_addrs. Con: mixes two concepts =
(load
balancing vs which address to try next on mismatch); semantics get
confusing.
Recommendation
Naming: Prefer try_all_addrs over check_all_addrs (clearer, same =
length). If
you want the link to session attrs in the name, use =
session_attrs_try_all_addrs
and accept the length.
Configuration: Keeping a separate connection parameter is the right =
design; the
main improvement is the name and a one-line note in the docs that it =
only has
effect when target_session_attrs is set. I wouldn't fold it into
target_session_attrs or into load_balance_hosts.

Made with Cursor.=

--Apple-Mail=_81011C1F-7E90-4DBF-8676-9005F0D8443B--